Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
github-actions[bot] closed pull request #60033: fix(providers/celery): Migrate conf imports to SDK compatibility layer URL: https://github.com/apache/airflow/pull/60033 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
github-actions[bot] commented on PR #60033: URL: https://github.com/apache/airflow/pull/60033#issuecomment-3981394971 This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
potiuk commented on PR #60033: URL: https://github.com/apache/airflow/pull/60033#issuecomment-3756252508 > Why do we need to add fallbacks to many configs? Did they not work without the fallbacks? @uranusjr -> This is manifestation of doing too much and in a wrong sequence during imports. The problem is that currently the sdk config does not have the defaults "hard-coded" fallbacks for celery and kubernetes providers (@amoghrajesh deliberately skipped them when moving conf to task.sdk) - because initialization of executors (that already need those fallbacks) is happening during importing airflow and before provider's manager initializes configuration retrieved from providers. This has been discussed and I proposed to bring it back as a temporary solution before we finish task isolation and will be able to do explicit initialization rather than import-based initialization: https://github.com/apache/airflow/pull/60074#issuecomment-3710183909 But maybe there are other proposals? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
uranusjr commented on PR #60033: URL: https://github.com/apache/airflow/pull/60033#issuecomment-3737255498 Why do we need to add fallbacks to many configs? Did they not work without the fallbacks? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
uranusjr commented on code in PR #60033: URL: https://github.com/apache/airflow/pull/60033#discussion_r2681163340 ## providers/celery/src/airflow/providers/celery/executors/celery_executor.py: ## Review Comment: There are unresolved conflict markers here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
ftakelait closed pull request #60033: fix(providers/celery): Migrate conf imports to SDK compatibility layer URL: https://github.com/apache/airflow/pull/60033 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
ftakelait commented on PR #60033: URL: https://github.com/apache/airflow/pull/60033#issuecomment-3732653583 > I have not seen your question - it's not visible here - I don't think @sunank200 is rather waiting for you - looking at the comments in PR. > > [#60074 (comment)](https://github.com/apache/airflow/pull/60074#issuecomment-3711902451) is where we discussed how to fix it - and hopefully it will be fixed there > > But you can still rebase though and keep on solving conflicts. That would be my recommendation while waiting for mechanism implemented for k8s -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
potiuk commented on PR #60033: URL: https://github.com/apache/airflow/pull/60033#issuecomment-3721129747 I have not seen your question - it's not visible here - I don't think @sunank200 is rather waiting for you - looking at the comments in PR. https://github.com/apache/airflow/pull/60074#issuecomment-3711902451 is where we discussed how to fix it - and hopefully it will be fixed there But you can still rebase though and keep on solving conflicts. That would be my recommendation while waiting for mechanism implemented for k8s -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
ftakelait commented on PR #60033: URL: https://github.com/apache/airflow/pull/60033#issuecomment-3720837836 > Still working on it @ftakelait ? are you going to rebase and fix the comments ? Hi @potiuk, I am still waiting for @sunank200 to reply about whether I should keep the `fallback` or I baypass the testing stage. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
potiuk commented on PR #60033: URL: https://github.com/apache/airflow/pull/60033#issuecomment-3720004752 Still working on it @ftakelait ? are you going to rebase and fix the comments ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix(providers/celery): Migrate conf imports to SDK compatibility layer [airflow]
sunank200 commented on code in PR #60033:
URL: https://github.com/apache/airflow/pull/60033#discussion_r2657556035
##
providers/celery/src/airflow/providers/celery/executors/celery_executor.py:
##
@@ -97,24 +96,24 @@ def __getattr__(name):
ARG_BROKER_API = Arg(("-a", "--broker-api"), help="Broker API")
ARG_FLOWER_HOSTNAME = Arg(
("-H", "--hostname"),
-default=conf.get("celery", "FLOWER_HOST"),
Review Comment:
Why do we need defaults?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
