-
Notifications
You must be signed in to change notification settings - Fork 79
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.
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
Source connections from workspace/deployment #1414
Merged
Merged
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
Looks good to me will need unit tests and a review from @jwitz for copy |
sunkickr
reviewed
Oct 12, 2023
sunkickr
reviewed
Oct 12, 2023
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1414 +/- ##
=======================================
Coverage 86.94% 86.95%
=======================================
Files 114 115 +1
Lines 13953 14033 +80
=======================================
+ Hits 12132 12202 +70
- Misses 1084 1092 +8
- Partials 737 739 +2
☔ View full report in Codecov by Sentry. |
kushalmalani
approved these changes
Oct 18, 2023
sunkickr
approved these changes
Oct 18, 2023
kushalmalani
added a commit
that referenced
this pull request
Oct 31, 2023
* Source connections from workspace/deployment * Working through unit tests * More * Temporarily comment out locally-broken test * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Test scenarios covered * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix lint issues * Remove new gitignore file * empty commit * add remove to test * fix lint * empty commit * put test back * fix test * Fixing test and cleanup test files * Disable feature by default * Enable feature for unit tests --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: David Koenitzer <davidkoenit@gmail.com> Co-authored-by: Kushal Malani <kushal@astronomer.io>
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.
Description
This change enables the CLI to source connections from the Astro workspace or deployment, in addition to the connections sourced from the
astro_settings.yaml
file. Note that the feature to add connections to a workspace or deployment is not yet available in Astro.When
astro dev start
orastro dev restart
is run with the new flag--deployment-id <Astro deployment id>
the connections will be retrieved for that deployment and added to the Airflow metadata DB in the same manner as for theastro_settings.yaml
file. If the deployment connections are inaccessible the start/restart will proceed without them.Similarly, if the new flag
--workspace-id <Astro workspace id>
is set, or otherwise the user has switched the CLI to an Astro workspace, the connections for that workspace will be loaded.The local file will take precedence if a connection id is present in both sources.
🧪 Functional Testing
📋 Checklist
make test
before taking out of draftmake lint
before taking out of draft