You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ephraimbuddy (via GitHub)" <gi...@apache.org> on 2023/03/08 14:54:11 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request, #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
ephraimbuddy opened a new pull request, #29978:
URL: https://github.com/apache/airflow/pull/29978
Time for `2.5.2rc1`!
--
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] pierrejeambrun commented on a diff in pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #29978:
URL: https://github.com/apache/airflow/pull/29978#discussion_r1130200950
##########
RELEASE_NOTES.rst:
##########
@@ -21,6 +21,113 @@
.. towncrier release notes start
+Airflow 2.5.2 (2023-03-13)
+--------------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+The date-time fields passed as API parameters or Params should be RFC3339-compliant (#29395)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+ In case of API calls, it was possible that "+" passed as part of the date-time fields were not URL-encoded, and
+ such date-time fields could pass validation. Such date-time parameters should now be URL-encoded (as ``%2B``).
+
+ In case of parameters, we still allow IS8601-compliant date-time (so for example it is possible that
+ ' ' was used instead of ``T`` separating date from time and no timezone was specified) but we raise
+ deprecation warning. (#29395)
Review Comment:
Indeed.
The output of:
```
towncrier build --version=${VERSION_WITHOUT_RC} --date=2023-03-14 --dir . --config newsfragments/config.toml
```
Is not `rst` formatted. So basically I end up editing and formatting this manually, to be consistent with previous releases.
This leave space for inconsistencies.
Same for the body of the changelog which comes from the following command, there is quite a few manual formatting operations here as well.
```
./dev/airflow-github changelog v2-5-stable v2-5-test
```
This is something I wanted to ask @ephraimbuddy, maybe there is a better way for building the release note I am not aware of.
--
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 #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1464156385
🎉
--
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 #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1461801986
FYI: `Build Images / Build Info (pull_request_target)` is not needed for v2-5-test push (it is normally skipped in Push) and it will get fixed right after we merge the branch.
Failing Breeze unit tests are becuase of **some** breeze PRs not cherry-picked (it shows that some of the configuration parameters are not properly configured).
Seems that the new isort requires python 3.8 but all our static checks are run with 3.7 so likely cherry-picking ruff changes is a better idea.
--
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 #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1460741564
@ashb We keep on having the same discussion with every release and it has never yet happened (for a good reason that we had only bugfix commits in any of the patchlevel releases).
I think we are all very well aware of those three:
```
A) semver
B) but keeping it to bug fixes we keep the release small -> learn like chance of breaking something with a bad cherry pick
C) we don't end up going down the path we had with 1.10 where we were cherry picking too make releases for two years
```
To apply to what gets released in the "apache-airflow" package.
But we also have to remember about all the things needed to release it and run tests in CI, dependencies that have been released in the meantime and failing our builds. Yes we can stop running tests. Yes we can stop improving our CI but there are depedencies relesed independently from airflow that make the old CI automation stop working (and cherry-picking the CI changes necessary to do it is the easy way to make the test pass.
Also we can update the documentation that has been improved for things that are already in the release and we put it in the hands of the users earlier.
None of those break semver, nor goes 1.10 path.
--
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 #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1460730946
@pierrejeambrun @ephraimbuddy we need to be much more picky about what makes it in to point releases. BUG FIXES ONLY please
I think we shall see in the change log - but looking at the list, there are many doc only fixes and dev tools /CI fixes.
--
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] pierrejeambrun commented on a diff in pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #29978:
URL: https://github.com/apache/airflow/pull/29978#discussion_r1130200950
##########
RELEASE_NOTES.rst:
##########
@@ -21,6 +21,113 @@
.. towncrier release notes start
+Airflow 2.5.2 (2023-03-13)
+--------------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+The date-time fields passed as API parameters or Params should be RFC3339-compliant (#29395)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+ In case of API calls, it was possible that "+" passed as part of the date-time fields were not URL-encoded, and
+ such date-time fields could pass validation. Such date-time parameters should now be URL-encoded (as ``%2B``).
+
+ In case of parameters, we still allow IS8601-compliant date-time (so for example it is possible that
+ ' ' was used instead of ``T`` separating date from time and no timezone was specified) but we raise
+ deprecation warning. (#29395)
Review Comment:
Indeed.
The output of:
```
towncrier build --version=${VERSION_WITHOUT_RC} --date=2023-03-14 --dir . --config newsfragments/config.toml
```
Is not `rst` formatted. Indent are list in fact that are removed. So basically we end up editing and formatting this manually, to be consistent with previous releases.
This leave space for inconsistencies. (PR numbers in title, underline, indents etc.)
Same for the body of the changelog which comes from the following command, there is quite a few manual formatting operations here as well.
```
./dev/airflow-github changelog v2-5-stable v2-5-test
```
This is something I wanted to ask @ephraimbuddy, maybe there is a better way for building the release note I am not aware of.
--
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] jedcunningham commented on a diff in pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on code in PR #29978:
URL: https://github.com/apache/airflow/pull/29978#discussion_r1130194491
##########
RELEASE_NOTES.rst:
##########
@@ -21,6 +21,113 @@
.. towncrier release notes start
+Airflow 2.5.2 (2023-03-13)
+--------------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+The date-time fields passed as API parameters or Params should be RFC3339-compliant (#29395)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+ In case of API calls, it was possible that "+" passed as part of the date-time fields were not URL-encoded, and
+ such date-time fields could pass validation. Such date-time parameters should now be URL-encoded (as ``%2B``).
+
+ In case of parameters, we still allow IS8601-compliant date-time (so for example it is possible that
+ ' ' was used instead of ``T`` separating date from time and no timezone was specified) but we raise
+ deprecation warning. (#29395)
Review Comment:
```suggestion
In case of API calls, it was possible that "+" passed as part of the date-time fields were not URL-encoded, and
such date-time fields could pass validation. Such date-time parameters should now be URL-encoded (as ``%2B``).
In case of parameters, we still allow IS8601-compliant date-time (so for example it is possible that
' ' was used instead of ``T`` separating date from time and no timezone was specified) but we raise
deprecation warning.
```
Don't need these indented (not sure why recent releases are, PR incoming), nor the PR number duplicated.
##########
RELEASE_NOTES.rst:
##########
@@ -21,6 +21,113 @@
.. towncrier release notes start
+Airflow 2.5.2 (2023-03-13)
+--------------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+The date-time fields passed as API parameters or Params should be RFC3339-compliant (#29395)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+ In case of API calls, it was possible that "+" passed as part of the date-time fields were not URL-encoded, and
+ such date-time fields could pass validation. Such date-time parameters should now be URL-encoded (as ``%2B``).
+
+ In case of parameters, we still allow IS8601-compliant date-time (so for example it is possible that
+ ' ' was used instead of ``T`` separating date from time and no timezone was specified) but we raise
+ deprecation warning. (#29395)
+
+Default for ``[webserver] expose_hostname`` changed to ``False`` (#29547)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+ The default for ``[webserver] expose_hostname`` has been set to ``False``, instead of ``True``. This means administrators must opt-in to expose webserver hostnames to end users. (#29547)
Review Comment:
```suggestion
The default for ``[webserver] expose_hostname`` has been set to ``False``, instead of ``True``. This means administrators must opt-in to expose webserver hostnames to end users.
```
##########
RELEASE_NOTES.rst:
##########
@@ -21,6 +21,113 @@
.. towncrier release notes start
+Airflow 2.5.2 (2023-03-13)
+--------------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+The date-time fields passed as API parameters or Params should be RFC3339-compliant (#29395)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
Review Comment:
```suggestion
""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
```
--
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 pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1460742366
I cherrypicked a lot of CI/breeze changes. Might have to drop them but let's get the changelog first
--
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] pierrejeambrun commented on pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1461011856
Static checks are failing because of isort hook having issue setting up. (ruff replaces isort on main).
I can fix that locally by upgrading isort hook to `5.11.4`. Should we go for this or bring back ruff PR in here ?
https://github.com/apache/airflow/actions/runs/4369131309/jobs/7642837979
--
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 #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1461632153
Thanks Pierre, and sorry to be a grouch about it
--
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 pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "ephraimbuddy (via GitHub)" <gi...@apache.org>.
ephraimbuddy commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1462070516
This ruff is added a lot of changes to the source. Wish there's an alternative fix for the isort issue
--
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] pierrejeambrun merged pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #29978:
URL: https://github.com/apache/airflow/pull/29978
--
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] jedcunningham commented on pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "jedcunningham (via GitHub)" <gi...@apache.org>.
jedcunningham commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1461016573
> upgrading isort hook to 5.11.4. Should we go for this or bring back ruff PR in here ?
I say we bump isort.
--
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 #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1460768056
In this case I'm not talking about the ci/breeze changes (however much I would like us not have to pick changes there every. single. release) but things like this
https://github.com/apache/airflow/pull/29978/commits/3784f2e25bda3c33b604212949d8bfca27924e94 - not only is it not a bug fix, it's not even to core!
I'm not budging on this one. There is to much in this point release that doesn't belong
--
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 #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1460881731
> In this case I'm not talking about the ci/breeze changes (however much I would like us not have to pick changes there every. single. release) but things like this
>
> [3784f2e](https://github.com/apache/airflow/commit/3784f2e25bda3c33b604212949d8bfca27924e94) - not only is it not a bug fix, it's not even to core!
Sure. We just spent quite some effort and time to look throuhg and help @ephraimbuddy and @pierrejeambrun to pin-point those issues rather than blank-stating that 180 issues is too much.
--
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] pierrejeambrun commented on a diff in pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #29978:
URL: https://github.com/apache/airflow/pull/29978#discussion_r1130205154
##########
RELEASE_NOTES.rst:
##########
@@ -21,6 +21,113 @@
.. towncrier release notes start
+Airflow 2.5.2 (2023-03-13)
+--------------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+The date-time fields passed as API parameters or Params should be RFC3339-compliant (#29395)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+ In case of API calls, it was possible that "+" passed as part of the date-time fields were not URL-encoded, and
+ such date-time fields could pass validation. Such date-time parameters should now be URL-encoded (as ``%2B``).
+
+ In case of parameters, we still allow IS8601-compliant date-time (so for example it is possible that
+ ' ' was used instead of ``T`` separating date from time and no timezone was specified) but we raise
+ deprecation warning. (#29395)
Review Comment:
Fixed
--
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] pierrejeambrun commented on a diff in pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #29978:
URL: https://github.com/apache/airflow/pull/29978#discussion_r1130200950
##########
RELEASE_NOTES.rst:
##########
@@ -21,6 +21,113 @@
.. towncrier release notes start
+Airflow 2.5.2 (2023-03-13)
+--------------------------
+
+Significant Changes
+^^^^^^^^^^^^^^^^^^^
+
+The date-time fields passed as API parameters or Params should be RFC3339-compliant (#29395)
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+ In case of API calls, it was possible that "+" passed as part of the date-time fields were not URL-encoded, and
+ such date-time fields could pass validation. Such date-time parameters should now be URL-encoded (as ``%2B``).
+
+ In case of parameters, we still allow IS8601-compliant date-time (so for example it is possible that
+ ' ' was used instead of ``T`` separating date from time and no timezone was specified) but we raise
+ deprecation warning. (#29395)
Review Comment:
Indeed.
The output of:
```
towncrier build --version=${VERSION_WITHOUT_RC} --date=2023-03-14 --dir . --config newsfragments/config.toml
```
Is not `rst` formatted. So basically I end up editing and formatting this manually, to be consistent with previous releases.
This leave space for inconsistencies. (PR numbers in title, underline etc.)
Same for the body of the changelog which comes from the following command, there is quite a few manual formatting operations here as well.
```
./dev/airflow-github changelog v2-5-stable v2-5-test
```
This is something I wanted to ask @ephraimbuddy, maybe there is a better way for building the release note I am not aware of.
--
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] pierrejeambrun commented on pull request #29978: Sync v2-5-stable with v2-5-test to release 2.5.2
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on PR #29978:
URL: https://github.com/apache/airflow/pull/29978#issuecomment-1460958522
> @pierrejeambrun @ephraimbuddy we need to be much more picky about what makes it in to point releases. BUG FIXES ONLY please
:+1:
The PR has been updated, thanks to the help of @jedcunningham @eladkal and @potiuk it has been trimmed down to 131 commits. (which is still a lot, but it has been a while since 2.5.1).
Changelog is ready, might help in reviewing. Just let me know if there are any adjustments to make :) (I expect CI failures)
--
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