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

Replace manifest loading from Artifactory #16

Merged
merged 13 commits into from
Oct 5, 2022
Merged

Replace manifest loading from Artifactory #16

merged 13 commits into from
Oct 5, 2022

Conversation

briantist
Copy link
Owner

@briantist briantist commented Oct 2, 2022

Closes #5

This is still a WIP, some things to clean up, and I want to generalize the upload code that's mostly duplicated between direct uploads and uploads-on-proxied-downloads.

Completely replaces the loading of the manifest from artifactory's archive file extraction.

Instead we borrowed stole the parsing from amanda (sivel/amanda@ca3f59f), and we now take the manifest out of the collection archive itself while it's local.

This solves two things for us:

  1. We stop relying on a paid feature of artifactory (Remove reliance on Artifactory archive file download #5), which:
    1. Lets people on Artifactory OSS make better use of galactory
    2. Brings us closer to actually writing and running integration tests for this project (Run integration tests against Artifactory #6)
  2. (minor) it stops skewing the downloads count in Artifactory -- this direct reading of files within archives seems increment the download count. In fact, Retries and performance improvements #9 reduced most of these requests by looking for the collection_info property before reading into the archive; before that change my downloads were in the 5 or 6 figures, because every iteration was triggering a "download". But this PR will reduce the last of those phantom downloads.

This PR will probably supersede #15 , but big thanks to @felipempda for posting that which pushed me to revisit this!

@briantist briantist added the enhancement New feature or request label Oct 2, 2022
@briantist briantist self-assigned this Oct 2, 2022
@briantist briantist marked this pull request as ready for review October 4, 2022 22:55
@briantist briantist merged commit 2e435d6 into main Oct 5, 2022
@briantist briantist deleted the manifest/tarz branch October 5, 2022 01:34
@briantist
Copy link
Owner Author

@felipempda there's no new release yet (I might create one within the next week or so), but if you're using the container you can pull the main tag to use this before then. Thanks again for the push to work on this! 😅

@felipempda
Copy link

Hahah thank YOU! I will be using it shortly probably in a WSGI server. Have a good day ;)

@briantist
Copy link
Owner Author

@felipempda do you have any public examples of using it with a WSGI server? Or do you have a preferred one? Even though Flask's server is not meant for production use, I've been fine with it so far, but I haven't tried using it with a full WSGI server yet and I would like to. I'm also considering whether it makes sense to publish additional containers using a specific WSGI server that are maybe more production ready.

@felipempda
Copy link

felipempda commented Oct 6, 2022

I know of puppetboard: https://github.com/voxpupuli/puppetboard
They have wsgi,py examples and setups, even their DockerFile use gunicorn (a WSGI server).
My challenge with current code is that there is no way to import an "application" WSGI ready.
I have to repeat a lot of what is in main.py in order to create a Flask "application" variable so that I can integrate with a WSGI server.
I know you wanted to use galactory as a script as well so that's the challenge.
I was thinking about proposing some changes to the project but it's new stuff for me as well.

@briantist
Copy link
Owner Author

Thanks, I will do some research on this over the coming months (might be more in the new year since Q4 is a very busy time), and see how that ight be made easier. I'll consult some other Flask-based projects and some colleagues who are more familiar with this kind of thing. Maybe the project can include a sort of library of integrations with different WSGI servers (or maybe that's an anti-pattern, will find out!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove reliance on Artifactory archive file download
2 participants