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

Justfile improvements #274

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open

Justfile improvements #274

wants to merge 27 commits into from

Conversation

malcolmgreaves
Copy link
Collaborator

@malcolmgreaves malcolmgreaves commented Oct 8, 2024

This PR adds several small improvements to the justfile. The build-{dev,release} commands will now skip the image build step if there is already an image with the same tag present locally.

The Justfile's setup command now logs the user into the NGC image repository. Login uses the NGC_API_KEY value from the user's .env file.

@malcolmgreaves malcolmgreaves self-assigned this Oct 8, 2024
@malcolmgreaves malcolmgreaves marked this pull request as draft October 8, 2024 00:25
@malcolmgreaves malcolmgreaves changed the title Adds --all to bionemo_data_download + --all to bionemo_data_download; build-{dev,release} skip if exists; setup docker NGC login Oct 8, 2024
@malcolmgreaves
Copy link
Collaborator Author

/build-ci

@pstjohn
Copy link
Collaborator

pstjohn commented Oct 8, 2024

Wait, why though? Do we really want users to pre-download all the test data?

Comment on lines 249 to 252
download_all = args.all
list_resources = args.list_resources
artifact_name = args.artifact_name
source = args.source
Copy link
Collaborator

Choose a reason for hiding this comment

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

you could even type annotate these

@malcolmgreaves malcolmgreaves changed the title + --all to bionemo_data_download; build-{dev,release} skip if exists; setup docker NGC login Justfile improvements and bionemo_data_download --all Oct 23, 2024
@malcolmgreaves malcolmgreaves marked this pull request as ready for review October 23, 2024 20:33
@malcolmgreaves malcolmgreaves changed the title Justfile improvements and bionemo_data_download --all Justfile improvements Oct 23, 2024
@malcolmgreaves
Copy link
Collaborator Author

@pstjohn @jstjohn @trvachov Moved the data loading stuff to its own PR: #358

@malcolmgreaves
Copy link
Collaborator Author

/build-ci

@malcolmgreaves
Copy link
Collaborator Author

/build-ci

@malcolmgreaves
Copy link
Collaborator Author

/build-ci

Copy link
Collaborator

@pstjohn pstjohn left a comment

Choose a reason for hiding this comment

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

LGTM!

I personally prefer mounting my local env files into the container rather than using the .env file -- maybe we could support that workflow sometime in down the road?

@malcolmgreaves
Copy link
Collaborator Author

For sure @pstjohn -- one thing that is convenient about justfiles is they can load .env files as variables into the justfile itself. This made it easier to adapt for our existing stuff. But I hear you that you want an option for volume mounting the file.

@malcolmgreaves
Copy link
Collaborator Author

/build-ci

@malcolmgreaves
Copy link
Collaborator Author

/build-ci

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.

5 participants