Connector Ops: Fix third party support in get_all_connectors_in_repo #31487
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
octavia-squidington-iii
added
the
area/documentation
| @@ -30,17 +30,17 @@ def test__get_type_and_name_from_technical_name(self, technical_name, expected_t | |||
| "connector, exists", | |||
| [ | |||
| (utils.Connector("source-faker"), True), | |||
| (utils.Connector("source-notpublished"), False), | |||
| (utils.Connector("source-doesnotexist"), False), | |||
There was a problem hiding this comment.
@alafanechere I think given where the code base is at, we should change this to an error
I.e. if you try to instantiate Connector but we cant find a metadata.yaml file. we throw an Error.
What are your thoughts?
There was a problem hiding this comment.
I'm in for this but I think this could break a couple of test.
I think that in some test scenario we provision a fake connector, without a metadata (or other file).
Feel free to make this change but please run the pipelines test suite to make sure nothing breaks. (I believe it will run automatically thanks to your recent change on the workflow).
There was a problem hiding this comment.
Will do! I think ill hold it for a subsequent PR
| if not self.has_airbyte_docs: | ||
| return None | ||
|
|
||
| return Path(f"{self.relative_documentation_path_str}.md") |
There was a problem hiding this comment.
Should we raise an error when has_airbyte_docs is True but path does not exists?
I would appreciate that the existence check logic move to this util instead of giving this responsability to the caller. Wdyt?
| if not self.has_airbyte_docs: | ||
| return None | ||
|
|
||
| return Path(f"{self.relative_documentation_path_str}.inapp.md") |
There was a problem hiding this comment.
Should we raise an error when has_airbyte_docs is True but path does not exists?
There was a problem hiding this comment.
Im unsure. A hard error would prevent the QA checks from running on a list of connectors as it would fail if just one was missing a documentation file. Is that a large concern?
Co-authored-by: Augustin <augustin@airbyte.io>
…irbytehq#31487) Co-authored-by: Augustin <augustin@airbyte.io>
Reopening of #31166
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