You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/01/05 18:03:04 UTC
[GitHub] [airflow] khalidmammadov opened a new pull request #20694: Code improvement to configuration file load process
khalidmammadov opened a new pull request #20694:
URL: https://github.com/apache/airflow/pull/20694
This improves configuration file load process by following:
- Removes code duplication
- Introduce single function that can load any config and provides default one when no config files specified
- Changes `conf` local variable to `_conf` to avoid confusion between global variable and local one
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20694: Code improvement to configuration file load process
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20694:
URL: https://github.com/apache/airflow/pull/20694#issuecomment-1005984178
I am always afraid to touch this part of code. You never know what's gonna blow when you do.
Let's wait for the tests to complete. Did you actually try to run some basic airflow commands @khalidmammadov - from my experience changes in conf often lead to airflow failing to start :D
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #20694: Code improvement to configuration file load process
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #20694:
URL: https://github.com/apache/airflow/pull/20694#issuecomment-1006916026
> To be clear the change only about code improvement and no logic was changed, I did make sure it does exactly the same things but with no duplicate code and hopefully bit more clear to understand/follow along.
Cool! Anyone else can review and confirm (we have the rule those kind of change - to the core - need two committers approvals @khalidmammadov )
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #20694: Code improvement to configuration file load process
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20694:
URL: https://github.com/apache/airflow/pull/20694#issuecomment-1006916433
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] khalidmammadov commented on pull request #20694: Code improvement to configuration file load process
Posted by GitBox <gi...@apache.org>.
khalidmammadov commented on pull request #20694:
URL: https://github.com/apache/airflow/pull/20694#issuecomment-1006897276
I have run whole test suite before and after change and no regression was detected. Also, run airflow scheduler and webserver and some dag runs and commands, again no issues.
To be clear the change only about code improvement and no logic was changed, I did make sure it does exactly the same things but with no duplicate code and hopefully bit more clear to understand/follow along.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #20694: Code improvement to configuration file load process
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20694:
URL: https://github.com/apache/airflow/pull/20694#issuecomment-1051386391
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] khalidmammadov closed pull request #20694: Code improvement to configuration file load process
Posted by GitBox <gi...@apache.org>.
khalidmammadov closed pull request #20694:
URL: https://github.com/apache/airflow/pull/20694
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org