Connector Ops: Fix third party support in get_all_connectors_in_repo #31166
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
octavia-squidington-iii
added
the
area/documentation
bnchrch
force-pushed
the
bnchrch/fix-third-party-support
branch
from
1d7f0d9 to
d986431
Compare
bnchrch
force-pushed
the
bnchrch/fix-third-party-support
branch
from
8bf1cc5 to
7a182d1
Compare
| def technical_name(self) -> str: | ||
| """ | ||
| Return the technical name of the connector from the given _relative_connector_path | ||
| e.g. source-google-sheets -> google-sheets or third-party/farosai/airbyte-pagerduty-source -> airbyte-pagerduty-source |
There was a problem hiding this comment.
| e.g. source-google-sheets -> google-sheets or third-party/farosai/airbyte-pagerduty-source -> airbyte-pagerduty-source | |
| e.g. source-google-sheets -> source-google-sheets or third-party/farosai/airbyte-pagerduty-source -> airbyte-pagerduty-source |
| @property | ||
| def name(self): | ||
| return self._get_type_and_name_from_technical_name()[1] | ||
|
|
||
| @property | ||
| def connector_type(self) -> str: | ||
| return self._get_type_and_name_from_technical_name()[0] | ||
| return self.metadata["connectorType"] |
There was a problem hiding this comment.
This question is outside the scope of this PR but I'm wondering if we should deserialize metadata with the metadata lib to interact with pydantic models instead of interacting with dict.
|
|
||
| @property | ||
| def documentation_directory(self) -> Path: | ||
| if self.has_external_documentation: | ||
| return None | ||
| return Path(f"./docs/integrations/{self.connector_type}s") |
There was a problem hiding this comment.
This .docs is one of the reason why airbyte-ci is required to run from the repo root. For more flexibility we shall add a constant like GIT_REPO_ROOT = git.Repo(search_parent_directories=True) and build path from this root.
There was a problem hiding this comment.
So I would have love to do this inside this PR but it got a bit big so I broke it out into this branch
#31409
Though it definately raises the question of if we would want to do this way....
The alternative is just having airbyte-ci set the working directory to the repo root 🤔
| @@ -297,7 +317,7 @@ def icon_path(self) -> Path: | |||
|
|
|||
| @property | |||
| def code_directory(self) -> Path: | |||
| return Path(f"./airbyte-integrations/connectors/{self.technical_name}") | |||
| return Path(f"./{CONNECTOR_PATH_PREFIX}/{self._relative_connector_path}") | |||
There was a problem hiding this comment.
Same suggestion as above: build the path from the GIT_REPO_ROOT
| for connector in all_connectors: | ||
| assert isinstance(connector, utils.Connector) | ||
| assert connector.metadata is not None | ||
| assert connector.documentation_file_path.exists() |
There was a problem hiding this comment.
This test would fail if a connector does not have a doc. Do we want this lib test to fail in this scenario? At the moment missing doc would lead to a failing QA check. What do you think about asserting the documention path structure instead of checking its existence?
There was a problem hiding this comment.
Good call. I updated to check if a connector has an airbyte.com documentation url before running this check
#31324
|
Accidentally approved and merge. ( I cant believe I typed that) I reverted the commit and reopened the PR |
…irbytehq#31166) Co-authored-by: bnchrch <[email protected]>
Problem
Third party pathing breaks
get_all_connectors_in_repoand theConnectorclassrelated to a comment from #30456
Solution
Connectorto take a relative path as its init argument