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/07/22 14:42:20 UTC

[GitHub] [airflow] potiuk opened a new pull request, #25236: Make tag fetching when preparing providers optional.

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

   Tag fetching is only needed to make sure we do not generate
   packages that have already been generated. However this is just
   an optimisation for CI runs. There is no harm if the tags are
   not refreshed and we generate the package again locally.
   
   Tag fetching might faile in various cases - for example when
   you are in corporate environment and require specific certificates
   to be available or when you are working from a worktree.
   
   After this change fetching tag will produce warning when there is
   an error and instruction on how you can fetch tags manually.
   
   <!--
   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 an 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 changes, an 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 a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
   


-- 
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 #25236: Make tag fetching when preparing providers optional.

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


##########
dev/provider_packages/prepare_provider_packages.py:
##########
@@ -710,10 +710,16 @@ def make_sure_remote_apache_exists_and_fetch(git_update: bool, verbose: bool):
         fetch_command.append("--unshallow")
     if verbose:
         console.print(f"Running command: '{' '.join(fetch_command)}'")
-    subprocess.check_call(
-        fetch_command,
-        stderr=subprocess.DEVNULL,
-    )
+    try:
+        subprocess.check_call(
+            fetch_command,
+        )
+    except subprocess.CalledProcessError as e:
+        console.print(
+            '[yellow]Error when fetching tags from remote. Your tags might not be refreshed. '
+            f'please refresh the tags manually via {" ".join(fetch_command)}\n'
+        )
+        console.print(f'[yellow]The error was: {e}')

Review Comment:
   This is something I am on the fence. It does not need end tag. It will close it automatically. But I usually like to have symmetry in any kind of markup. But seems that I subconsciously omit it more and more, so maybe a time to change habits :)



-- 
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] josh-fell commented on a diff in pull request #25236: Make tag fetching when preparing providers optional.

Posted by GitBox <gi...@apache.org>.
josh-fell commented on code in PR #25236:
URL: https://github.com/apache/airflow/pull/25236#discussion_r927899851


##########
dev/provider_packages/prepare_provider_packages.py:
##########
@@ -710,10 +710,16 @@ def make_sure_remote_apache_exists_and_fetch(git_update: bool, verbose: bool):
         fetch_command.append("--unshallow")
     if verbose:
         console.print(f"Running command: '{' '.join(fetch_command)}'")
-    subprocess.check_call(
-        fetch_command,
-        stderr=subprocess.DEVNULL,
-    )
+    try:
+        subprocess.check_call(
+            fetch_command,
+        )
+    except subprocess.CalledProcessError as e:
+        console.print(
+            '[yellow]Error when fetching tags from remote. Your tags might not be refreshed. '
+            f'please refresh the tags manually via {" ".join(fetch_command)}\n'
+        )
+        console.print(f'[yellow]The error was: {e}')

Review Comment:
   Does this need an end-formmating tag? Not sure if we'd expect the `[yellow]` formatting to continue. RIch is new to me so really more curiosity.



##########
dev/provider_packages/prepare_provider_packages.py:
##########
@@ -710,10 +710,16 @@ def make_sure_remote_apache_exists_and_fetch(git_update: bool, verbose: bool):
         fetch_command.append("--unshallow")
     if verbose:
         console.print(f"Running command: '{' '.join(fetch_command)}'")
-    subprocess.check_call(
-        fetch_command,
-        stderr=subprocess.DEVNULL,
-    )
+    try:
+        subprocess.check_call(
+            fetch_command,
+        )
+    except subprocess.CalledProcessError as e:
+        console.print(
+            '[yellow]Error when fetching tags from remote. Your tags might not be refreshed. '
+            f'please refresh the tags manually via {" ".join(fetch_command)}\n'

Review Comment:
   ```suggestion
               f'Please refresh the tags manually via {" ".join(fetch_command)}\n'
   ```



-- 
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 #25236: Make tag fetching when preparing providers optional.

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


-- 
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 #25236: Make tag fetching when preparing providers optional.

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

   cc: @PatrykKlimowicz 


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