Make documentation_url optional in a declarative connector spec #21347
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
girarda
approved these changes
Jan 12, 2023
There was a problem hiding this comment.
lgtm. next steps would be to update the changelog and update the CDK version before releasing the change
|
|
||
| def generate_spec(self) -> ConnectorSpecification: | ||
| """ | ||
| Returns the connector specification according the spec block defined in the low code connector manifest. | ||
| """ | ||
|
|
||
| obj = {"connectionSpecification": self.connection_specification} | ||
|
|
||
| if self.documentation_url is not None and self.documentation_url != '': |
There was a problem hiding this comment.
nit: this could be simplified to
if self.documentation_url:
jbfbell
pushed a commit
that referenced
this pull request
Jan 13, 2023
* Make documentation_url optional in a declarative connector spec * simplify if statement * bump patch version of cdk * Revert "bump patch version of cdk" This reverts commit 1854bf3. * 🤖 Bump patch version of Airbyte CDK * Revert "🤖 Bump patch version of Airbyte CDK" This reverts commit 85d5a98. * 🤖 Bump patch version of Airbyte CDK Co-authored-by: lmossman <lmossman@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Make the
documentation_urlfield of a declarative connector's spec optional. This is because users will not always have a documentation url to set on low-code connectors, and the documentation url is already optional in the airbyte protocol, so this brings the declarative CDK in alignment with that.See slack thread for context: https://airbytehq-team.slack.com/archives/C03TP6W8081/p1673474183580999?thread_ts=1673463486.033949&cid=C03TP6W8081
How
Make
documentation_urlnon-required in the declarative component schema, as well as in the declarative spec.py file.This has been tested on a low-code connector that doesn't set the documentation_url, as well as one that sets it to the empty string, and both cases work properly with
python main.py specreturning the Spec that does not contain the documentation URL.It was also tested that a low-code connector which does set the documentation_url still has that url being returned as part of the spec for the above command.