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/02/13 12:00:22 UTC

[GitHub] [airflow] potiuk opened a new pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

potiuk opened a new pull request #21546:
URL: https://github.com/apache/airflow/pull/21546


   We are preparing to switch from Buster to Bullseye and this is
   the second change that is needed (following #21522). This change
   allows to choose whether we want to use Buster or Bullseye images
   as a base. We need to be able to choose, because:
   
   1) we want to keep backwards compatibility and continue our
      users to build Buster-base images
   2) we cannot yet fully switch to Bullseye because MsSQL's odbc
      driver does not yet support Bullseye and we reached out to
      mysql maintainers to learn about their plans to make the
      decision on when and how we are going to support Bullseye and
      MSSQL.
   
      Details of this discussion are in:
      https://github.com/MicrosoftDocs/sql-docs/issues/7255#issuecomment-1037097131
   
   This PR adds the capability of choosing the DEBIAN_VERSION in
   Breeze when building images but does not yet switch from Buster to
   Bullseye
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ 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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   cc: @Bowrna  - you will need to also add `--debian-version` switch to `Breeze2` when we merge this 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] potiuk commented on a change in pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21546:
URL: https://github.com/apache/airflow/pull/21546#discussion_r815205278



##########
File path: tests/jobs/test_scheduler_job.py
##########
@@ -3902,13 +3902,16 @@ def test_process_dags_queries_count(
 
             failures = []  # Collects assertion errors and report all of them at the end.
             message = "Expected {expected_count} query, but got {current_count} located at:"
-            for expected_query_count in expected_query_counts:
-                with create_session() as session:
-                    try:
-                        with assert_queries_count(expected_query_count, message):
-                            self.scheduler_job._do_scheduling(session)
-                    except AssertionError as e:
-                        failures.append(str(e))
+            # in Debian Bullseye for MSSQL one of the tests gets into additional loop
+            # More often, and we should increase the interval of fetching to avoid that
+            with conf_vars({('core', 'min_serialized_dag_fetch_interval'): '30'}):

Review comment:
       Right. That is strange. I think this might be a temporary effect. I will take a close look at the #21378 when we actually swtich.




-- 
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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   > I guess this is okay,
   > 
   > I'm just not sure that we even need it configurable -- why not just switch it rather than add all the complexity of making it configurable and dealing with both dists?
   
   Because I want the users to be able to build the image based on buster at least till it reaches the end of life. This is what I suggested as our policy. The example with MSSQL where they released Bullseye-compatible libraries only 3 weeks ago shows that some of our users might want to be be able to build the images on their own if they need to stay with Buster.


-- 
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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   We can easily merge that one now. The #21378 will actually make the switch.


-- 
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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   


-- 
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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   Anyone :) ?


-- 
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 a change in pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21546:
URL: https://github.com/apache/airflow/pull/21546#discussion_r815201542



##########
File path: tests/jobs/test_scheduler_job.py
##########
@@ -3902,13 +3902,16 @@ def test_process_dags_queries_count(
 
             failures = []  # Collects assertion errors and report all of them at the end.
             message = "Expected {expected_count} query, but got {current_count} located at:"
-            for expected_query_count in expected_query_counts:
-                with create_session() as session:
-                    try:
-                        with assert_queries_count(expected_query_count, message):
-                            self.scheduler_job._do_scheduling(session)
-                    except AssertionError as e:
-                        failures.append(str(e))
+            # in Debian Bullseye for MSSQL one of the tests gets into additional loop
+            # More often, and we should increase the interval of fetching to avoid that
+            with conf_vars({('core', 'min_serialized_dag_fetch_interval'): '30'}):

Review comment:
       On L3889 We set the value to 100 (seconds), so this setting here actually _decreases_ the timeout, so this comment doesn't quite hold up.




-- 
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 edited a comment on pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #21546:
URL: https://github.com/apache/airflow/pull/21546#issuecomment-1054255314


   This is what I proposed a "lazy consensus" on and it is already merged in our README: 
   
   https://github.com/apache/airflow/blob/main/README.md#base-os-support-for-reference-airflow-images
   
   > Users will continue to be able to build their images using stable Debian releases until the end of life and building and verifying of the images happens in our CI but no unit tests are executed using this image in the main branch.


-- 
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 closed pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   


-- 
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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   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] ashb commented on pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   Ah cool


-- 
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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   This is what I proposed a "lazy consensus" on and it is already merged in our README: 
   
   https://github.com/apache/airflow/blob/main/README.md#base-os-support-for-reference-airflow-images


-- 
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 closed pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   


-- 
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 #21546: Allow to switch easily between Bullseye and Buster debian versions

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


   cc: @Bowrna  - you will need to also add `--debian-version` switch to `Breeze2` when we merge this 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] potiuk commented on a change in pull request #21546: Allow to switch easily between Bullseye and Buster debian versions

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #21546:
URL: https://github.com/apache/airflow/pull/21546#discussion_r815308589



##########
File path: tests/jobs/test_scheduler_job.py
##########
@@ -3902,13 +3902,16 @@ def test_process_dags_queries_count(
 
             failures = []  # Collects assertion errors and report all of them at the end.
             message = "Expected {expected_count} query, but got {current_count} located at:"
-            for expected_query_count in expected_query_counts:
-                with create_session() as session:
-                    try:
-                        with assert_queries_count(expected_query_count, message):
-                            self.scheduler_job._do_scheduling(session)
-                    except AssertionError as e:
-                        failures.append(str(e))
+            # in Debian Bullseye for MSSQL one of the tests gets into additional loop
+            # More often, and we should increase the interval of fetching to avoid that
+            with conf_vars({('core', 'min_serialized_dag_fetch_interval'): '30'}):

Review comment:
       Yeah. Seem's that removing that change did not change anything in #21378 :)




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