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

Make pushsource compatible with attrs version 22.2.0 #295

Merged
merged 1 commit into from Jan 10, 2023

Conversation

querti
Copy link
Collaborator

@querti querti commented Jan 5, 2023

Since the attrs version release, pub tests have begun failing with the error "TypeError: keywords must be strings". Attrs have added an "alias" parameter to each attribute[1], which is used in the attrs's "evolve" method to construct a new instance. This causes issues with the attribute "from", which has some special logic implemented in order to be usable at all[2]. The attribute "from" is generated from "from_", copying all its parameters. Since "alias" is a new parameter, it isn't copied and thus its value is set no "None". attrs's "evolve" method uses **kwargs to create a new instance, and it sets one of the keys in **kwargs to "None" (from alias). Keys in **kwargs must be strings, which causes the TypeError. It can be fixed by also setting "alias" in the newly created "from" attribute. In case of "from_", it has the same value as "name" parameter ("from_"), which is why we don't want to copy it from the old attribute, but explicitly set it to "from" (otherwise it would be "from_"). The change should be backwards compatible with older versions of attrs.

[1] python-attrs/attrs#950
[2] #108

Refers to CLOUDDST-17044

@querti querti marked this pull request as ready for review January 5, 2023 10:04
Since new attrs version release, pub tests have begun failing with the
error "TypeError: keywords must be strings". Attrs have added an "alias"
parameter to each attribute[1], which is used in the attrs's "evolve"
method to construct a new instance. This causes issues with the attribute
"from", which has some special logic implemented in order to be usable
at all[2]. The attribute "from" is generated from "from_", copying all
its parameters. Since "alias" is a new parameter, it isn't copied and
thus its value is set no "None". attrs's "evolve" method uses **kwargs
to create a new instance, and it sets one of the keys in **kwargs to
"None" (from alias). Keys in **kwargs must be strings, which causes the
TypeError. It can be fixed by also setting "alias" in the newly created
"from" attribute. In case of "from_", it has the same value as "name"
parameter ("from_"), which is why we don't want to copy it from the old
attribute, but explicitly set it to "from" (otherwise it would be
"from_"). The change should be backwards compatible with older versions
of attrs.

[1] python-attrs/attrs#950
[2] release-engineering#108
@querti
Copy link
Collaborator Author

querti commented Jan 5, 2023

@release-engineering/distribution pls review

@codecov
Copy link

codecov bot commented Jan 5, 2023

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (3449f5d) compared to base (81ec605).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #295   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           49        49           
  Lines         1974      1977    +3     
=========================================
+ Hits          1974      1977    +3     
Impacted Files Coverage Δ
src/pushsource/_impl/model/erratum_fixup.py 100.00% <100.00%> (ø)
src/pushsource/_impl/compat_attr.py 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@querti querti merged commit 9e919dc into release-engineering:master Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants