-
Notifications
You must be signed in to change notification settings - Fork 41
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
Help with moving bandersnatch to python3.12 #172
Comments
Hi @cooperlees As long that we don't have a lot of open issues to manage I'm OK with keeping this issue as a platform for helping you guys plan the refactor. The beauty in s3path is that you don't need to know S3, you just need to think about it like a normal file system and use the regular pathlib API. Now about your code, in general I don't see any reason to keep your our S3Path object. Except for that all looks greate resource, _ = path._accessor.configuration_map.get_configuration(path) In python 3.12 it will brake import s3path
resource, _ = s3path.configuration_map.get_configuration(path) |
I tried to do this like you've said (Please correct on PR if I misuderstood) in regards to configuration_map but it seems to not hold authentication settings when using it this way.
Do we need a public API to do this with auth settings correct? Or does bandersnatch's CI need to set auth settings more than ENV variables and in the test initialization: |
Hi @cooperlees About the issue that you have with boto credentials, can you give me a unit test (without your ecosystem) to understand that is the issue here? |
Thanks for the response. I guess moto mocks more than we do here. I don't know how to simplify the error situation, but I feel it's pretty clear it's something that works when we use the private instances that have been initialized with the auth settings we need vs. your public methods that do not. We just setup auth for minio and hit it via s3 APIs, the integration testing isn't that complex I feel for someone who understands s3 and s3path (which is not me). I'll go try update to 0.5.7 and see if that helps, but I don't think it will. All I want to do is move bandersnatch to 3.12, and s3path has made that difficult :( |
Sadly same errors with 0.5.7 - I'm not sure what I am meant to do differently with it |
@cooperlees |
Ok, good news. I sat down a read your PRs for 0.5.7 and saw what I misunderstood and we got further! I was able to move to the new public API everywhere (missed a few but they are in the PR linked below). I now even have CI "passing" in a 3.12 environment, so I thank you for all your help.
I think we only have 1 remaining big item (apart from other unitest private usages I'll look into separately after working this out) tho. I'm sure you don't want us to be using
|
Hi,
I am the maintainer of bandersnatch, but I have very little s3 skills in general - so would love help moving to >= 3.12 + S3Path and cleaning up our usage of s3path which I feel we hacked around issues rather than fixing bugs here when it was contributed.
Code
pypa/bandersnatch#1710
Feel free to close if you feel this is an abuse of using issues. I'd also love a list of how you'd approach the move if you don't have time to do it, as I am confused in the order I should potentially take and if there is any hope maintaining backwards compatibility or if I should give up with that and just move to >= 3.12 only for S3 support?
The text was updated successfully, but these errors were encountered: