-
-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Replace credentials-secret with db/broker connection(s) #743
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
base: main
Are you sure you want to change the base?
Conversation
docs/modules/airflow/examples/getting_started/code/airflow.yaml
Outdated
Show resolved
Hide resolved
docs/modules/airflow/examples/getting_started/code/airflow.yaml
Outdated
Show resolved
Hide resolved
docs/modules/airflow/examples/getting_started/code/airflow.yaml
Outdated
Show resolved
Hide resolved
docs/modules/airflow/examples/getting_started/code/airflow.yaml
Outdated
Show resolved
Hide resolved
maltesander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First batch of comments.
tests/templates/kuttl/versioning/30-install-airflow-cluster.yaml.j2
Outdated
Show resolved
Hide resolved
| # TODO revert this before merging! | ||
| # - name: logging | ||
| # dimensions: | ||
| # - airflow | ||
| # - openshift | ||
| # - executor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
marker!
| uriSecret: redis-celery # <3> | ||
| ---- | ||
|
|
||
| <1> A reference to a secret which must contain the single fields `uri` e.g. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel URI is very generic. I think connectionString is a better description?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong feelings - I just went with what we had in the decision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same, @sbernauer any opinions?
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
…cluster.yaml.j2 Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
…uster.yaml.j2 Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
…ml.j2 Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
…r.yaml.j2 Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
maltesander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more doc reviews. Tests look good.
Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
…l.j2 Co-authored-by: Malte Sander <malte.sander.it@gmail.com>
|
Re-ran tests after latest changes 🟢 https://testing.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/airflow-operator-it-custom/77/ |
|
We (me, @sbernauer and @maltesander) discussed this today and decided to move it to tracking. The PR is "finished" in the sense that the implementation is complete, but there are improvements we can make to the code (e.g. using traits) to make it more flexible for other products that will use this.
|
Description
Fixes #64
Decision: https://github.com/stackabletech/decisions/issues/72
Notes for reviewers
database.rs,queue.rsandconnection.rs.$FOO) by processes started from the shell${env:...}and has been written to a config fileenvlibrary is not safe in a multithreaded environmentJenkins 🟢 https://testing.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/airflow-operator-it-custom/75/
Openshift 🟢 https://testing.stackable.tech/view/02%20Operator%20Tests%20(custom)/job/airflow-operator-it-custom/76/
Important
The logging test has been deactivated temporarily while testing this PR. Revert the setting before final approval/merging.
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker