-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add ability to disable apiserver from the helm chart using apiServer enabled #56493
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
Conversation
Miretpl
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.
In general change looks good! Could you also add test cases for it?
d8985f8 to
c4b3dfe
Compare
|
Sure! Just added some tests |
|
Change looks good to me, but some tests are failing. Could you take a look? |
amoghrajesh
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.
Mostly looks good, some comments re quality checks and also the static checks are failing, could you look?
cf430b3 to
6e02cc2
Compare
|
Resolved change requests and static checks are now passing. |
Miretpl
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.
Hi @dstandish @jedcunningham @hussein-awala @amoghrajesh could you take a look?
amoghrajesh
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.
nits regarding flipping the checks but otherwise looks good.
|
Can this be merged? |
jscheffl
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.
I thnk it can. But let's have CI turning green.
…enabled (apache#56493) Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
…enabled (apache#56493) Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
…enabled (apache#56493) Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
…enabled
From #56492, add the ability to disable the api server so that we can create a deployment that only creates workers (for remote execution with airflow 3.0+)