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

Bugfix 589 master_v3.1 docker data #590

Merged
merged 5 commits into from
Sep 1, 2020

Conversation

JohnHalleyGotway
Copy link
Collaborator

Please see this note describing the changes:
#589 (comment)

I ran the script, created the images, and pushed them to DockerHub here:https://hub.docker.com/repository/docker/dtcenter/metplus-data/tags?page=1

… point directory for each tarball so that you can mount multiple ones in a docker run command without them clobbering eachother.
@JohnHalleyGotway
Copy link
Collaborator Author

JohnHalleyGotway commented Sep 1, 2020

George, if/when we merge these changes, I'll make the same changes and submit a PR for develop.

Copy link
Collaborator

@georgemccabe georgemccabe left a comment

Choose a reason for hiding this comment

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

This is great you were able to consolidate the Dockerfiles into one customizable file.
One comment regarding the same changes in develop: There are some changes I added in @fentoad72 's fork that involve consolidating the list of sample data tarball names into 1 file under ci/travis_jobs. The list in metplus_sample_data is mostly duplicated from this file. You can make the same changes in develop as you did for master_v3.1, but we should be sure to reconcile the two lists so that they is only one list to change.

… that includes the source METplus version to avoid confusion with the current METplus version which is being used to create the Docker tag.
Copy link
Collaborator

@georgemccabe georgemccabe left a comment

Choose a reason for hiding this comment

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

Looks good.

@JohnHalleyGotway
Copy link
Collaborator Author

I updated this PR to add a header block to the script and add -push/-version command line options to override the defaults. Also, read the metplus_sample_data file from the directory in which the script being run ($0) lives. That way you don't have to run the build_docker_images.sh from inside the directory in which it lives. Here's how you could run it and automatically push results to DockerHub:

build_docker_images.sh -version 4.0 -push

Copy link
Collaborator

@georgemccabe georgemccabe left a comment

Choose a reason for hiding this comment

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

New arguments look good to me and will be useful to create a new image on DockerHub

@JohnHalleyGotway JohnHalleyGotway merged commit 3083610 into master_v3.1 Sep 1, 2020
@JohnHalleyGotway JohnHalleyGotway deleted the bugfix_589_master_v3.1_docker_data branch September 2, 2020 16:05
@JohnHalleyGotway JohnHalleyGotway linked an issue Sep 2, 2020 that may be closed by this pull request
19 tasks
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.

Add missing Dockerfiles for the data assets of the METplus 3.1 release.
2 participants