Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing pydantic v1.10.0a1 #77246

Closed
wants to merge 2 commits into from
Closed

testing pydantic v1.10.0a1 #77246

wants to merge 2 commits into from

Conversation

samuelcolvin
Copy link

TO NOT MERGE

This is pull request is just to test pydantic v1.10, see pydantic/pydantic#4359

@probot-home-assistant probot-home-assistant bot added core small-pr PRs with less than 30 lines. labels Aug 24, 2022
@homeassistant
Copy link
Contributor

Hi @samuelcolvin,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@@ -117,7 +117,7 @@ backoff<2.0

# Breaking change in version
# https://github.com/samuelcolvin/pydantic/issues/4092
pydantic!=1.9.1
pydantic=1.10.0a1
Copy link
Member

@frenck frenck Aug 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not perse testing it?
It just loosens a constrain, but doesn't install this particular version?
So unless, a package we use installs 1.10.0a1 in their requirements, nothing will happen.

Please note, Home Assistant doesn't directly use pydantic, some of our dependencies do.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I've corrected the constraint)

It looks like SystemBridgeCoordinatorData is the only pydantic model in this repo.

If you think there's no value in running these tests, feel free to close this.

If dependants of this repo use pydantic, I'll try to test them directly, but if dependencies use pydantic, running these tests would be good if possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SystemBridgeCoordinatorData

Hmm, that seems incorrect / a bug, as that integration doesn't specify having a dependency on pedantic.

If you think there's no value in running these tests, feel free to close this.

It was not a question about value :) I was a question about this not doing what you probably expected it would do; which is running our test suite against pydantic 1.10.0a1.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, in that case close this.

By the way, it's probably worth excluding pydantic>2 since pydantic V2 will break a lot of things.

Copy link
Member

@frenck frenck Aug 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is up to the dependencies to handle correctly. We only add constraints when needed and have no better way out.

@frenck frenck marked this pull request as draft August 24, 2022 08:47
@homeassistant
Copy link
Contributor

Hi @samuelcolvin,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@samuelcolvin samuelcolvin deleted the test-pydantic-v1.10 branch August 24, 2022 08:58
@github-actions github-actions bot locked and limited conversation to collaborators Aug 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-needed core small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants