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

refactor: consolidate f strings for sha file and images url #74

Merged
merged 2 commits into from
Aug 22, 2024

Conversation

williams-jack
Copy link
Collaborator

Feature/Problem Description

Passing around a grader image's SHA Sum (for downloading and checking against local file system) was unclear and redundant when calling the retrieve_image_tgz_from_url function. This PR seeks to consolidate that logic.

Solution (Changes Made)

  • Function now takes in just the SHA sum.
  • File name generated once, and then used for creating the URL.

Copy link
Collaborator

@blerner blerner left a comment

Choose a reason for hiding this comment

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

I think you've picked slightly the wrong abstraction-boundary. In the image_loading.py file, it doesn't care that the dockerfile you're looking for has a sha-based name. All it should do is care that "if you give me whatever you think is a unique name, I'll handle the details of looking for and downloading the corresponding .tgz file, and looking for and loading the corresponding image

worker/orca_grader/__main__.py Outdated Show resolved Hide resolved
worker/orca_grader/docker_utils/images/image_loading.py Outdated Show resolved Hide resolved
@blerner blerner merged commit 4213bb9 into main Aug 22, 2024
1 of 2 checks passed
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.

2 participants