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

Fix URL quoting for collection info #72

Merged
merged 5 commits into from
Jul 5, 2023
Merged

Fix URL quoting for collection info #72

merged 5 commits into from
Jul 5, 2023

Conversation

briantist
Copy link
Owner

@briantist briantist commented Jun 29, 2023

Fixes: #58
Fixes: #52
Closes: #54
Closes: #59
Closes: #71

With devopshq/artifactory#409 merged, the underlying issue is fixed upstream, we just need to consume it by setting the parameter appropriately since it currently defaults to previous behavior.

This PR does that, and to avoid guard code, it also sets the minimum version of that library to the newly released v0.9.0 .

Huge thanks to @jcox10 @mamercad for raising the issue and doing the heavy troubleshooting!


Important

The malformation of collection info happened on publish, which, for collections uploaded (cached) via proxying, will have happened silently and then affected subsequent pulls.

The fix in this PR cannot fix collections which have already been published in artifactory with malformed collection_info, so errors will still be seen trying to retrieve those.

The collection info could be manually repaired, but the recommended fix is to republish.

For collections that were not proxied

I recommend re-publishing the collection (and probably delete the existing ones beforehand).

For collections that were proxied

I recommend deleting all (or all affected) proxied collections, and letting them get re-populated naturally on request.

@briantist briantist added bug Something isn't working dependencies Pull requests that update a dependency file labels Jun 29, 2023
@briantist briantist added this to the v0.10.0 milestone Jun 29, 2023
@briantist briantist self-assigned this Jun 29, 2023
@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.11 ⚠️

Comparison is base (95a604f) 56.70% compared to head (891d1fb) 56.59%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
- Coverage   56.70%   56.59%   -0.11%     
==========================================
  Files          26       26              
  Lines         947      947              
==========================================
- Hits          537      536       -1     
- Misses        410      411       +1     
Impacted Files Coverage Δ
galactory/upstream.py 24.27% <0.00%> (ø)
galactory/utilities.py 44.89% <0.00%> (-0.69%) ⬇️

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

@jcox10
Copy link
Contributor

jcox10 commented Jun 30, 2023 via email

@briantist
Copy link
Owner Author

Thanks @jcox10 , you should be able to test this out from my branch:

pip install https://github.com/briantist/galactory/archive/fix/url-quoting.tar.gz

It would be great to get some additional local testing before merging (I'll be doing some tests too).

@briantist
Copy link
Owner Author

Hey @jcox10 I'm going to be wrapping up my testing soon and will look to merge this unless I find any glaring issues. If you have a chance to try it out that'd be cool :)

@briantist briantist merged commit 525321c into main Jul 5, 2023
11 checks passed
@briantist briantist deleted the fix/url-quoting branch July 5, 2023 17:54
@jcox10
Copy link
Contributor

jcox10 commented Jul 12, 2023

Sorry for the delay, been really busy. I finally tested it out by pulling the latest main and then turning off GALACTORY_USE_PROPERTY_FALLBACK. The windows collection I previously had issues with downloaded and the properties were set correctly without truncation! I think its working properly.

@briantist
Copy link
Owner Author

No worries @jcox10 , I'm very grateful for the feedback and additional confirmation. I plan to release this in v0.10.0 once #77 is finished (which should be very soon).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSON error getting ansible.windows collection versions Traceback when publishing amazon.aws
2 participants