Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ProtocolVersion to StandardDefs #16237

Merged
merged 5 commits into from
Sep 2, 2022
Merged

Add ProtocolVersion to StandardDefs #16237

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d586cd3
Add ProtocolVersion to StandardDefs
gosusnp Sep 1, 2022
741db48
Fix letter case
gosusnp Sep 1, 2022
16d022c
Merge branch 'master' into gosusnp/16201-add_protocol_version_to_acto…
gosusnp Sep 1, 2022
8abed04
Merge branch 'master' into gosusnp/16201-add_protocol_version_to_acto…
gosusnp Sep 1, 2022
927e639
Merge branch 'master' into gosusnp/16201-add_protocol_version_to_acto…
gosusnp Sep 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions airbyte-config/config-models/src/main/resources/types/StandardDestinationDefinition.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,6 @@ properties:
format: date
resourceRequirements:
"$ref": ActorDefinitionResourceRequirements.yaml
protocol_version:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just passing by, seems like we're using camelCase for most of these - should it be protocolVersion here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I think I saw a few comments about prefering snake_case over camelCase for the attributes in the models.
The generated output remains camelCase for the Java objects.
No strong opinion here.

Copy link
Contributor

@pedroslopez pedroslopez Sep 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah - main reason why I pointed this out is since at least specifically within these StandardXYZDefinitions schemas it seems like camel case is being used. Also no strong opinion, just think we should try to be consistent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go for file consistency there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same conclusion, change on the way

type: string
description: the Airbyte Protocol version supported by the connector
3 changes: 3 additions & 0 deletions airbyte-config/config-models/src/main/resources/types/StandardSourceDefinition.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,6 @@ properties:
format: date
resourceRequirements:
"$ref": ActorDefinitionResourceRequirements.yaml
protocol_version:
type: string
description: the Airbyte Protocol version supported by the connector
4 changes: 4 additions & 0 deletions ...e-config/config-persistence/src/main/java/io/airbyte/config/persistence/ConfigWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ static void writeStandardSourceDefinition(final List<StandardSourceDefinition> c
: Enums.toEnum(standardSourceDefinition.getSourceType().value(),
SourceType.class).orElseThrow())
.set(Tables.ACTOR_DEFINITION.SPEC, JSONB.valueOf(Jsons.serialize(standardSourceDefinition.getSpec())))
.set(Tables.ACTOR_DEFINITION.PROTOCOL_VERSION, standardSourceDefinition.getProtocolVersion())
.set(Tables.ACTOR_DEFINITION.TOMBSTONE, standardSourceDefinition.getTombstone())
.set(Tables.ACTOR_DEFINITION.PUBLIC, standardSourceDefinition.getPublic())
.set(Tables.ACTOR_DEFINITION.CUSTOM, standardSourceDefinition.getCustom())
Expand Down Expand Up @@ -79,6 +80,7 @@ static void writeStandardSourceDefinition(final List<StandardSourceDefinition> c
: Enums.toEnum(standardSourceDefinition.getSourceType().value(),
SourceType.class).orElseThrow())
.set(Tables.ACTOR_DEFINITION.SPEC, JSONB.valueOf(Jsons.serialize(standardSourceDefinition.getSpec())))
.set(Tables.ACTOR_DEFINITION.PROTOCOL_VERSION, standardSourceDefinition.getProtocolVersion())
.set(Tables.ACTOR_DEFINITION.TOMBSTONE, standardSourceDefinition.getTombstone() != null && standardSourceDefinition.getTombstone())
.set(Tables.ACTOR_DEFINITION.PUBLIC, standardSourceDefinition.getPublic())
.set(Tables.ACTOR_DEFINITION.CUSTOM, standardSourceDefinition.getCustom())
Expand Down Expand Up @@ -115,6 +117,7 @@ static void writeStandardDestinationDefinition(final List<StandardDestinationDef
.set(Tables.ACTOR_DEFINITION.ICON, standardDestinationDefinition.getIcon())
.set(Tables.ACTOR_DEFINITION.ACTOR_TYPE, ActorType.destination)
.set(Tables.ACTOR_DEFINITION.SPEC, JSONB.valueOf(Jsons.serialize(standardDestinationDefinition.getSpec())))
.set(Tables.ACTOR_DEFINITION.PROTOCOL_VERSION, standardDestinationDefinition.getProtocolVersion())
.set(Tables.ACTOR_DEFINITION.TOMBSTONE, standardDestinationDefinition.getTombstone())
.set(Tables.ACTOR_DEFINITION.PUBLIC, standardDestinationDefinition.getPublic())
.set(Tables.ACTOR_DEFINITION.CUSTOM, standardDestinationDefinition.getCustom())
Expand All @@ -140,6 +143,7 @@ static void writeStandardDestinationDefinition(final List<StandardDestinationDef
.set(Tables.ACTOR_DEFINITION.ICON, standardDestinationDefinition.getIcon())
.set(Tables.ACTOR_DEFINITION.ACTOR_TYPE, ActorType.destination)
.set(Tables.ACTOR_DEFINITION.SPEC, JSONB.valueOf(Jsons.serialize(standardDestinationDefinition.getSpec())))
.set(Tables.ACTOR_DEFINITION.PROTOCOL_VERSION, standardDestinationDefinition.getProtocolVersion())
.set(Tables.ACTOR_DEFINITION.TOMBSTONE,
standardDestinationDefinition.getTombstone() != null && standardDestinationDefinition.getTombstone())
.set(Tables.ACTOR_DEFINITION.PUBLIC, standardDestinationDefinition.getPublic())
Expand Down
2 changes: 2 additions & 0 deletions ...te-config/config-persistence/src/main/java/io/airbyte/config/persistence/DbConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ public static StandardSourceDefinition buildStandardSourceDefinition(final Recor
.withSourceType(record.get(ACTOR_DEFINITION.SOURCE_TYPE) == null ? null
: Enums.toEnum(record.get(ACTOR_DEFINITION.SOURCE_TYPE, String.class), SourceType.class).orElseThrow())
.withSpec(Jsons.deserialize(record.get(ACTOR_DEFINITION.SPEC).data(), ConnectorSpecification.class))
.withProtocolVersion(record.get(ACTOR_DEFINITION.PROTOCOL_VERSION, String.class))
.withTombstone(record.get(ACTOR_DEFINITION.TOMBSTONE))
.withPublic(record.get(ACTOR_DEFINITION.PUBLIC))
.withCustom(record.get(ACTOR_DEFINITION.CUSTOM))
Expand All @@ -121,6 +122,7 @@ public static StandardDestinationDefinition buildStandardDestinationDefinition(f
.withDocumentationUrl(record.get(ACTOR_DEFINITION.DOCUMENTATION_URL))
.withName(record.get(ACTOR_DEFINITION.NAME))
.withSpec(Jsons.deserialize(record.get(ACTOR_DEFINITION.SPEC).data(), ConnectorSpecification.class))
.withProtocolVersion(record.get(ACTOR_DEFINITION.PROTOCOL_VERSION, String.class))
.withTombstone(record.get(ACTOR_DEFINITION.TOMBSTONE))
.withPublic(record.get(ACTOR_DEFINITION.PUBLIC))
.withCustom(record.get(ACTOR_DEFINITION.CUSTOM))
Expand Down
5 changes: 5 additions & 0 deletions airbyte-config/config-persistence/src/test/java/io/airbyte/config/persistence/MockData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ public static StandardSourceDefinition publicSourceDefinition() {
.withDocumentationUrl("documentation-url-1")
.withIcon("icon-1")
.withSpec(connectorSpecification())
.withProtocolVersion("0.2.1")
.withTombstone(false)
.withPublic(true)
.withCustom(false)
Expand Down Expand Up @@ -206,6 +207,7 @@ public static StandardSourceDefinition grantableSourceDefinition2() {
.withDockerImageTag("tag-3")
.withDockerRepository("repository-3")
.withDocumentationUrl("documentation-url-3")
.withProtocolVersion("0.2.2")
.withIcon("icon-3")
.withTombstone(false)
.withPublic(false)
Expand All @@ -220,6 +222,7 @@ public static StandardSourceDefinition customSourceDefinition() {
.withDockerImageTag("tag-4")
.withDockerRepository("repository-4")
.withDocumentationUrl("documentation-url-4")
.withProtocolVersion("0.2.4")
.withIcon("icon-4")
.withTombstone(false)
.withPublic(false)
Expand Down Expand Up @@ -256,6 +259,7 @@ public static StandardDestinationDefinition publicDestinationDefinition() {
.withDocumentationUrl("documentation-url-3")
.withIcon("icon-3")
.withSpec(connectorSpecification())
.withProtocolVersion("0.3.1")
.withTombstone(false)
.withPublic(true)
.withCustom(false)
Expand Down Expand Up @@ -299,6 +303,7 @@ public static StandardDestinationDefinition cusstomDestinationDefinition() {
.withDocumentationUrl("documentation-url-44")
.withIcon("icon-4")
.withSpec(connectorSpecification())
.withProtocolVersion("0.3.2")
.withTombstone(false)
.withPublic(false)
.withCustom(true);
Expand Down