airbyte-ci: improve QA checks for airbyte-enterprise #45393
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
postamar
changed the title
postamar
changed the title
| @@ -163,6 +164,8 @@ def __init__(self, context: ConnectorContext) -> None: | |||
| context=context, | |||
| paths_to_mount=[ | |||
| MountPath(code_directory), | |||
| # Support airbyte-enterprise docs. | |||
| MountPath(SUBMODULE_ENTERPRISE_DOCS_PATH), | |||
There was a problem hiding this comment.
this is just a handful of markdown files so this should be quite cheap
postamar
force-pushed
the
postamar/airbyte-ci-improve-qa-checks-for-airbyte-enterprise
branch
from
f8ea69e to
efdb757
Compare
| @@ -163,6 +164,8 @@ def __init__(self, context: ConnectorContext) -> None: | |||
| context=context, | |||
| paths_to_mount=[ | |||
| MountPath(code_directory), | |||
| # Support airbyte-enterprise docs. | |||
| MountPath(SUBMODULE_ENTERPRISE_DOCS_PATH), | |||
There was a problem hiding this comment.
I'm not sure how/if this mount path is symlinked to the main doc path.
The way we define the connector.documentation_file_path attribute is by:
- Keeping the path portion after
https//docs.airbyte.com/<the path relative to .docs> - Appending it to
./docs
If the symlink works I think we're good.
There was a problem hiding this comment.
the symlink works in dagger if both the symlink and its target are mounted
|
Running a simple connector test suite on this branch to check if QA checks continue to work normally: |
airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/common.py
Outdated
Show resolved
Hide resolved
postamar
force-pushed
the
postamar/airbyte-ci-improve-qa-checks-for-airbyte-enterprise
branch
2 times, most recently
from
56a6b38 to
ceebfe0
Compare
postamar
force-pushed
the
postamar/airbyte-ci-improve-qa-checks-for-airbyte-enterprise
branch
from
ceebfe0 to
e416af2
Compare
|
This is ready for another look @alafanechere . I tested it in both repos and it works beautifully. |
| container = container.with_mounted_directory(destination_path, self.context.get_repo_dir(path_string)) | ||
| if path_to_mount.get_path().is_symlink(): | ||
| container = self._mount_path(container, path_to_mount.get_path().readlink()) | ||
|
|
There was a problem hiding this comment.
isn't an else missing there?
There was a problem hiding this comment.
no, if it's not a symlink, then line 61 is enough
There was a problem hiding this comment.
But line 61 will remount the path without resolution if the path is a symlink, you mean we want to mount both in any case right?
There was a problem hiding this comment.
L61 mounts the symlink itself. It's one of the quirks of buildkit that it doesn't resolve the symlink before mounting it.
There was a problem hiding this comment.
we're not the only people to be annoyed by this: moby/moby#1676
|
@postamar I'm not seeing the change you originally made to the QA checks step to mount the submodule |
I removed it deliberately, because it's no longer needed. Mounting a |
Beautiful!!! |
|
Running the test on a light connector again. https://github.com/airbytehq/airbyte/actions/runs/10817241987 |
Co-authored-by: Augustin <augustin@airbyte.io>
|
I'd also kicked off a run here: https://github.com/airbytehq/airbyte/actions/runs/10817143202/job/30009903241 edit: success |
|
Thanks for the speedy review! |
|
Our friend the pull rate limit is back! I'll re-run and merge tomorrow morning. |
postamar
deleted the
postamar/airbyte-ci-improve-qa-checks-for-airbyte-enterprise
branch
What
This PR makes it possible for airbyte-ci to run in airbyte-enterprise with symlinks instead of actual docs.
This is a companion to https://github.com/airbytehq/airbyte-enterprise/pull/43 and is required by it for it to pass CI. See its description for context.
How
Improves symlink support in airbyte-ci.
Review guide
n/a
User Impact
None
Can this PR be safely reverted and rolled back?