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

Compress and download model #1883

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Rukongai
Copy link
Contributor

Checklist

🚨 Please review the guidelines for contributing to this repository. 🚨

  • Make sure you are making a pull request against our main branch (left side)
  • Check that that your branch is up to date with our main.
  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your main!
  • Check that the tests and code linter both pass.
  • If you're a new contributor, please sign our contributor license agreement.

Warnings

  • This PR will change existing database contents.
  • This PR introduces a breaking change to existing installations.

Summary

Download an entire model as a single zip file

Linked issues

#1847

Description of changes

Added a new controller for managing file downloads, a route, and button.

Needs to be refactored, cleaned up, and has bugs, but the outline is starting to appear

To Do:
Add some sort of progress or activity indicator
Delete temporary files after download
Figure out bug where larger archives don't get all the files transferred over
Fix bug where previous downloaded model is included in new download (naming + cleaning tmp will help this)
Refactor & cleanup

@Floppy
Copy link
Collaborator

Floppy commented Feb 18, 2024

This is cool! I was thinking about this a while back and it would be good to have the zipfile hang around in a temp folder somewhere that will get cleared out on an LRU basis, so that popular downloads don't have to be zipped up every time. Not sure exactly how that would work best though, hadn't got much further than the general idea.

@Rukongai
Copy link
Contributor Author

i'll see what I can figure out. I'm not a programmer by trade and the first time I touched Ruby was 2 days ago, so I'm just figuring stuff out as I go along.

@Floppy
Copy link
Collaborator

Floppy commented Feb 18, 2024

I'm well impressed for two days of Ruby! Don't worry about that too much, it's more longer-term planning on my part. Certainly just the on-demand compression that you have here would be fine as a starter.

private

# A helper method to make the recursion work.
def write_entries(entries, path, zipfile)
Copy link

Choose a reason for hiding this comment

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

Method write_entries has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.

Copy link

codeclimate bot commented Feb 29, 2024

Code Climate has analyzed commit b25c3d6 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 41.1% (50% is the threshold).

This pull request will bring the total coverage in the repository to 72.8% (-0.9% change).

View more on Code Climate.

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

Successfully merging this pull request may close these issues.

None yet

2 participants