-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
CI: Add job for pip check
#64058
CI: Add job for pip check
#64058
Conversation
I'm hesitant about adding a job that we know will fail, but since it will only run for requirement bumps, I think it will still provide useful information. I'd like to hear what Franck and others think. |
For now. At least my idea was that we work towards fixing the existing problems, since this will be necessary anyway for the new pip resolver in #59769. Over the last few days, I've already addressed a couple of them with one currently pending #64045. AFAIK there are a few options we have to resolve them, some better some worse.
-- Conflicting dependenciesOpen PRs
Waiting for release
Open Issues
Discontinued
Requires HA dependency update
Long wait / possibly unmaintained
Pending dependency update
Current conflicts
|
@MartinHjelmare I though about what you said about an always failing job a bit more. Maybe we could get away with comparing the result to a predefined constant / environment variable. I.e. we expect 15 conflicts, fail only if there are more than 15. That would also allow us to run it on changes to the There is a small issue about what to do if a conflict gets resolved. Ideally, and what I've implemented in the last commit, the expected conflicts variable should be updated. Otherwise it would be possible to introduce a new conflict in another PR. To notice this case, I've chosen to emit an error for it too. Let me know what you think. |
Sounds good! One limitation with pip check that we should be aware of, is that a new conflict won't necessarily be caused by the updated requirement. It could instead be due to an existing requirement that has a bad pinning strategy and the updated requirement just crossed that bad pinning limit. |
Good point! I agree. My hope is that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might work for now 👍
I would like to merge it now. We can always come back to it later for fine-tuning after we got some more experience with it on a day by day basis. |
Proposed change
This PR adds a new CI job which runs
pip check
if any requirements change.I had the idea after @MartinHjelmare mentioned a dependency conflict in #64045 (review) which I hadn't seen at the time.
Due to existing dependency conflicts, this job will always fail at the moment. The output should nevertheless help not to add additional issues. The job is only run for PRs which include requirement changes. It doesn't make much sense anywhere else. Especially not with it failing by default.
Edit: I've reworked the logic to only fail if a new dependency conflict is added. More details here: #64058 (comment)
This could also be helpful for #59769
/CC: @MartinHjelmare @frenck
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: