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

Use address as stable key when batching field sets in lint/test #18725

Merged
merged 3 commits into from
Apr 13, 2023

Conversation

danxmoran
Copy link
Contributor

Closes #18566

Using str(x) as the stable batching key ends up using repr when x is a field set. After #18719 this no longer raises an exception, but it's still an unintended behavior change introduced as part of the new partitioning logic in 2.15.x.

Revert back to using .address as the partitioning key for field sets, and add regression tests.

These tests would have previously errored out, but were fixed by pantsbuild#18719.
Adding them now before I change the batching key logic to be sure I
don't re-break anything.
Closes pantsbuild#18566

Using `str(x)` as the stable batching key ends up using `repr` when `x`
is a field set. This previously raised an exception in some cases - it's
also an unintended behavior change introduced as part of the new
partitioning logic in 2.15.x. Revert back to using `.address` as the
partitioning key for field sets.
@danxmoran danxmoran added needs-cherrypick category:bugfix Bug fixes for released features labels Apr 12, 2023
@danxmoran danxmoran added this to the 2.15.x milestone Apr 12, 2023
@danxmoran danxmoran requested a review from thejcannon April 12, 2023 18:00
@danxmoran danxmoran requested a review from thejcannon April 12, 2023 20:06
@danxmoran danxmoran merged commit 21b992e into pantsbuild:main Apr 13, 2023
@danxmoran danxmoran deleted the danxmoran/fix-partition-key branch April 13, 2023 14:37
danxmoran added a commit to danxmoran/pants that referenced this pull request Apr 13, 2023
…antsbuild#18725)

Closes pantsbuild#18566

Using `str(x)` as the stable batching key ends up using `repr` when `x`
is a field set. After pantsbuild#18719 this no longer raises an exception, but
it's still an unintended behavior change introduced as part of the new
partitioning logic in 2.15.x.

Revert back to using `.address` as the partitioning key for field sets,
and add regression tests.
danxmoran added a commit to danxmoran/pants that referenced this pull request Apr 13, 2023
…antsbuild#18725)

Closes pantsbuild#18566

Using `str(x)` as the stable batching key ends up using `repr` when `x`
is a field set. After pantsbuild#18719 this no longer raises an exception, but
it's still an unintended behavior change introduced as part of the new
partitioning logic in 2.15.x.

Revert back to using `.address` as the partitioning key for field sets,
and add regression tests.
danxmoran added a commit that referenced this pull request Apr 13, 2023
…Cherry-pick of #18725) (#18733)

Closes #18566

Using `str(x)` as the stable batching key ends up using `repr` when `x`
is a field set. After #18719 this no longer raises an exception, but
it's still an unintended behavior change introduced as part of the new
partitioning logic in 2.15.x.

Revert back to using `.address` as the partitioning key for field sets,
and add regression tests.
danxmoran added a commit that referenced this pull request Apr 13, 2023
…Cherry-pick of #18725) (#18734)

Closes #18566

Using `str(x)` as the stable batching key ends up using `repr` when `x`
is a field set. After #18719 this no longer raises an exception, but
it's still an unintended behavior change introduced as part of the new
partitioning logic in 2.15.x.

Revert back to using `.address` as the partitioning key for field sets,
and add regression tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bugfix Bug fixes for released features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We semi-broke partitioners for targets
3 participants