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.

Sign up for GitHub

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

Jump to bottom

GH-467: fix ArtifactoryPath.__reduce__ for python 3.9 and below #468

Merged
merged 5 commits into from
Jan 16, 2025
Merged

GH-467: fix ArtifactoryPath.__reduce__ for python 3.9 and below #468

merged 5 commits into from
Jan 16, 2025

Conversation

miskeens
Copy link
Contributor

Fixes an issue with DOHQ CrossPM (which was caused indirectly).

Closes #467

@miskeens
Copy link
Contributor Author

@allburov @beliaev-maksim hi! Requesting a review since devopshq/crosspm on python before 3.10 suffers from #456

allburov
allburov reviewed Jan 13, 2025
View reviewed changes
@miskeens miskeens marked this pull request as draft January 14, 2025 06:40
@allburov allburov marked this pull request as ready for review January 14, 2025 08:29
allburov
allburov approved these changes Jan 14, 2025
View reviewed changes
Copy link
Member

@allburov allburov left a comment

Choose a reason for hiding this comment

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

LGTM!

@miskeens miskeens marked this pull request as draft January 14, 2025 09:51
@miskeens
Copy link
Contributor Author

set PR to draft as the changes still cause failure. copy.deepcopy() called in CrossPM still requires object to copy to have __dict__ property. pushing new solution soon

@miskeens
Copy link
Contributor Author

not sure how to check reduce() on python 3.9 and lower, but these changes resolve the CrossPM issue.

@miskeens miskeens marked this pull request as ready for review January 14, 2025 10:22
@beliaev-maksim
Copy link
Member

Is there a way to add some test in this repo?

@miskeens
Copy link
Contributor Author

miskeens commented Jan 14, 2025 •
edited
Loading

Is there a way to add some test in this repo?

not that I know of currently :) I did not author the changes for pickling, but I don't see testing ArtifactoryPath deepcopying as a crucial requirement at this point

all I'm interested in is getting CrossPM to work ASAP

@allburov allburov merged commit 176e67b into devopshq:master Jan 16, 2025
5 checks passed
@miskeens
Copy link
Contributor Author

@allburov should we expect a new release soon?

@allburov
Copy link
Member

We're doing release 21 Jan 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allburov allburov allburov approved these changes

@beliaev-maksim beliaev-maksim Awaiting requested review from beliaev-maksim

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ArtifactoryPath.__reduce__() method raises an AttributeError on Python 3.9 and below
3 participants
@miskeens @beliaev-maksim @allburov