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 2021/07/05 13:02:39 UTC

[GitHub] [airflow] uranusjr opened a new pull request #16814: Automatically create section when migrating config

uranusjr opened a new pull request #16814:
URL: https://github.com/apache/airflow/pull/16814


   Previously, if a config is migrated to a new section, the migration code would crash with NoSectionError if the user does not add that section to airflow.cfg after upgrading Airflow. This patch automatically creates an empty section when that happens to avoid Airflow from crashing.
   


-- 
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] ashb commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-875552930


   @kaxil Ohh, I see.
   
   So the section exists in the default_config, but when we to the upgrade and try to _write_ the config back , the sections are only checked in the top level config, cos that's where we are writing to.


-- 
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 merged pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #16814:
URL: https://github.com/apache/airflow/pull/16814


   


-- 
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 #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-912491920


   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] kaxil commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-874123512


   And for the all configs that don't exist in user's airflow.cfg we default to Airflow's default config (which is separate than user's airflow.cfg)
   
   https://github.com/apache/airflow/blob/b251d22fffad63124eec5246b80035408b543704/airflow/configuration.py#L359


-- 
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] kaxil commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-874122910






-- 
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] kaxil commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-874122910


   > Previously, if a config is migrated to a new section, the migration code would crash with NoSectionError if the user does not add that section to airflow.cfg after upgrading Airflow. This patch automatically creates an empty section when that happens to avoid Airflow from crashing.
   
   It should not error, as we have deprecations for the configs/sections that are moved:
   
   https://github.com/apache/airflow/blob/b251d22fffad63124eec5246b80035408b543704/airflow/configuration.py#L105-L185
   


-- 
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] ashb commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-888366088


   > and a unit test should be added to cover the section-only-in-default-config-not-user-config case. Is that right?
   
   Yup, that's right.


-- 
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] uranusjr commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-888133625


   Trying to figure out what needs to be done here. So the patch is not wrong (although my description to the issue was, and a unit test should be added to cover the section-only-in-default-config-not-user-config case. Is that right?


-- 
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] ashb commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-887437218


   @uranusjr Ping - I think this is worth finishing off.


-- 
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] ashb commented on pull request #16814: Automatically create section when migrating config

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #16814:
URL: https://github.com/apache/airflow/pull/16814#issuecomment-875582015


   @uranusjr We should probably expand the unit tests to cover this case


-- 
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