Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Jun 30, 2022

The "provider_dependencies.json" file gets automatically generated
by pre-commit and in case it cannot be json-parsed, it will break
building the CI image. This is a bit chicken-egg because on
CI the image is needed to run pre-commits that could warn the
user this is the case (and the image fails build with a bit
cryptic message).

This PR improves the diagnostics:

  • it runs the pre-commit check before image building which will
    fix the generated file (and will let the build run - only to
    fail at the latest static-checks step

  • it prints warning to the user seeing pre-commit error to not
    modify the file manually but let pre-commit do the job.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragement file, named {pr_number}.significant.rst, in newsfragments.

The "provider_dependencies.json" file gets automatically generated
by pre-commit and in case it cannot be json-parsed, it will break
building the CI image. This is a bit chicken-egg because on
CI the image is needed to run pre-commits that could warn the
user this is the case (and the image fails build with a bit
cryptic message).

This PR improves the diagnostics:

* it runs the pre-commit check before image building which will
  fix the generated file (and will let the build run - only to
  fail at the latest static-checks step

* it prints warning to the user seeing pre-commit error to not
  modify the file manually but let pre-commit do the job.
@potiuk potiuk force-pushed the add-better-dependencies-diagnostics branch from a943509 to 1daff1c Compare June 30, 2022 21:47
@potiuk
Copy link
Member Author

potiuk commented Jun 30, 2022

By looking at the output I realized two things were missing:

  • caching of the env (shaving - 20-30 seconds for initialization in most cases)
  • switching on coloured ouptut and showing diff on error.

Both are added @eladkal

@potiuk potiuk merged commit 1b3905e into main Jul 1, 2022
@potiuk potiuk deleted the add-better-dependencies-diagnostics branch May 6, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants