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/12/02 21:21:33 UTC

[GitHub] [airflow] ephraimbuddy opened a new pull request, #28076: Update main with 2.5.0 config values

ephraimbuddy opened a new pull request, #28076:
URL: https://github.com/apache/airflow/pull/28076

   Because of the change of section, these keys are being updated as if they were added in 2.5.0. These are suggestions from `./dev/validate_version_added_fields_in_config.py`


-- 
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] ephraimbuddy closed pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
ephraimbuddy closed pull request #28076: Update main with 2.5.0 config values
URL: https://github.com/apache/airflow/pull/28076


-- 
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] ephraimbuddy commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1040450956


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   `AIRFLOW__KUBERNETES__POD_TEMPLATE_FILE` was 1.10.11 while `AIRFLOW__KUBERNETES_EXECUTOR__POD_TEMPLATE_FILE` started existing in 2.5.0. 
   Makes me wonder if we should have a new field on the config file, `version_renamed` or something because, in some way, this was really added in 2.5.0 but practically works down to 1.10.11 with old section where it was originally added in the old section.
   cc @ashb 



-- 
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] ephraimbuddy commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1045545595


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   > BTW. Glad you are back :). I hope you rested 👍 We missed you 😄
   
   Thanks! Missed you guys too 😀



-- 
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] ephraimbuddy commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1040450956


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   `AIRFLOW__KUBERNETES__POD_TEMPLATE_FILE` was 1.10.11 while `AIRFLOW__KUBERNETES_EXECUTOR__POD_TEMPLATE_FILE` started existing in 2.5.0. 
   Makes me wonder if we should have a new field on the config file, `version_renamed` or something because, in some way, this was really added in 2.5.0 but practically works down to 1.10.11 where it was originally added in the old section.
   cc @ashb 



-- 
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 a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1045531308


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   I think it's also misleading now. 
   
   We should add a feature IMHO (because as far as I know it does not exist now) to  refer to which was deprecated/previous version of the config - and still keep the "from" there.  It does not have to be detailed but it should explain that  this section has been renamed from [kubernetes] in 2.5.0 



-- 
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] XD-DENG commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1038592653


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   (Many others below too. This one is just an example)



-- 
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] ephraimbuddy commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1045434699


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   I have been off for a while... @potiuk what do you think about this? I think the correctness depends on how one looks at it. The section/option is new in 2.5.0 while the old section/option is not new. 
   
   If we mark it 1.10.11, users would think it'll work in 1.10.11 but that's not true. It will only work in 1.10.11 with the old section name, not the new one.
   
   



-- 
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] XD-DENG commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1040444614


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   I do not think so. This change will leave new-comers an impression that these configurations are really new since 2.5.0, while it's not the truth



-- 
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 a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1045539027


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   BTW. Glad you are back :). I hope you rested 👍  We missed you 😄 



-- 
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] ephraimbuddy commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1038743142


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   Yeah, this is not nice...section renaming



-- 
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] XD-DENG commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
XD-DENG commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1038592312


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   I don't think we should change these values?



-- 
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] ephraimbuddy commented on a diff in pull request #28076: Update main with 2.5.0 config values

Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #28076:
URL: https://github.com/apache/airflow/pull/28076#discussion_r1040443295


##########
airflow/config_templates/config.yml:
##########
@@ -2377,7 +2377,7 @@
         failed worker pods will not be deleted so users can investigate them.
         This only prevents removal of worker pods where the worker itself failed,
         not when the task it ran failed.
-      version_added: 1.10.11
+      version_added: 2.5.0

Review Comment:
   Ok. I think this is the correct thing to do. We have moved the configurations to a new section. The new section was created in 2.5.0, therefore, it makes sense that all items in the section are also added in 2.5.0. This was the practice when we moved some items to a new 'database' section. See https://github.com/apache/airflow/pull/22284. WDYT
   



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