Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
potiuk commented on PR #58952: URL: https://github.com/apache/airflow/pull/58952#issuecomment-3604097911 Backport in https://github.com/apache/airflow/pull/58977 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
potiuk commented on PR #58952: URL: https://github.com/apache/airflow/pull/58952#issuecomment-3604076665 @johnslavik Actually - we even have the right method - we just do not call it in `start-airflow` . It even reads the version from .pre-commit-config.yaml ``` def assert_prek_installed(): """ Check if prek is installed in the right version. :return: True is the prek is installed in the right version. ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
potiuk commented on PR #58952: URL: https://github.com/apache/airflow/pull/58952#issuecomment-3604063584 > In both scenarios the script assumes it's waiting for a prek job that started successfully and could fail, not for a job that didn't start (and I think we can dry-run / check that before launching everything else, since if prek fails then the entire machinery is destined to fail at some point -- otherwise we'd get a warning, not an error) Yes - we could. We already do that for other things so checking for prek (but specifically when prek should be used) is a good idea. https://github.com/apache/airflow/blob/main/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py#L539 This is actually a very important point to fix those things and give as good of a message for all those edge cases as it makes it easier for new contributos. We usually discover **always** something when new person starts using things and we usually relentlessly fix 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
github-actions[bot] commented on PR #58952: URL: https://github.com/apache/airflow/pull/58952#issuecomment-3604057702 ### Backport failed to create: v3-1-test. View the failure log Run details Status Branch Result ❌ v3-1-test https://github.com/apache/airflow/commit/b95fca4dbdf2775919e9c01ad8a56da5a4b46af2";> You can attempt to backport this manually by running: ```bash cherry_picker b95fca4 v3-1-test ``` This should apply the commit to the v3-1-test branch and leave the commit in conflict state marking the files that need manual conflict resolution. After you have resolved the conflicts, you can continue the backport process by running: ```bash cherry_picker --continue ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
potiuk merged PR #58952: URL: https://github.com/apache/airflow/pull/58952 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
johnslavik commented on PR #58952: URL: https://github.com/apache/airflow/pull/58952#issuecomment-3602900675 When the prek version is lower than required, the error message is now much more helpful: ``` mpleAuthManager detected. All roles (admin, viewer, user, op) are always available via configuration in .dev/breeze/src/airflow_breeze/files/simple_auth_manager_passwords.json The asset compilation failed. Exiting. error: Failed to parse `.pre-commit-config.yaml` caused by: Required minimum prek version `0.2.0` is greater than current version `0.1.1`. Please consider updating prek. at line 18 column 1 Error 1 returned ``` (previously it was just `No hook found with id `compile-ui-assets` in stage manual`) Maybe in a followup PR we could make `breeze start-airflow` fail early though? In both scenarios the script assumes it's waiting for a job that started successfully and could fail, not a for a job that didn't start. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
johnslavik commented on PR #58952: URL: https://github.com/apache/airflow/pull/58952#issuecomment-3602869662 :rocket: https://github.com/user-attachments/assets/8eae014a-2e7a-4960-9c5c-99d905150b5e"; /> -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Bump prek lower bound to 0.2.0 [airflow]
johnslavik commented on PR #58952: URL: https://github.com/apache/airflow/pull/58952#issuecomment-3602825724 `skip common compat check` I guess? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
[PR] Bump prek lower bound to 0.2.0 [airflow]
johnslavik opened a new pull request, #58952:
URL: https://github.com/apache/airflow/pull/58952
Otherwise `breeze start-airflow` can't start Airflow from top-level as it
relies on nested `.pre-commit-config.yml` discovery that's been introduced in
prek 0.2.0 (["Workspace
Mode"](https://github.com/j178/prek/blob/3ff564e8bfe0e8afb65ff5c427df8cbbb988f94a/CHANGELOG.md#020)).
I'll mark as ready after confirming the minimum version works like a breeze.
---
**^ Add meaningful description above**
Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.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+Improvement+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
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
