-
Notifications
You must be signed in to change notification settings - Fork 59
Update docker file #467
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
Update docker file #467
Conversation
User can use docker run command to run the tool. Update the integration tests to be based on the new image.
Pull Request Test Coverage Report for Build 1662
💛 - Coveralls |
cloudbuild_CI.yaml
Outdated
| args: | ||
| - 'build' | ||
| - '--build-arg' | ||
| - 'IMAGE_TAG=${COMMIT_SHA}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The downside of this is that even our released versions will not use the release tag, they will still use the commit SHA1.
I'm not sure there is a better option, though.
cloudbuild_CI.yaml
Outdated
| # The only reason that we use the built image in the 'name' argument is | ||
| # convenience, i.e., to avoid installing python, virtualenv, etc. on the | ||
| # image. | ||
| # Run the test script from the very image we built and pushed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know that this comment adds any value?
cloudbuild_CI.yaml
Outdated
| - '--run_all_tests' | ||
| - '--test_name_prefix cloud-ci-' | ||
| id: 'test-gcp-variant-transforms-docker' | ||
| entrypoint: './deploy_and_run_tests.sh' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ./ gives me pause. I'd usually expect to see either the bare script (to allow exec to do PATH lookup) or a full path.
It's not clear to me why this works (eg, this suggests the working directory inside the container contains the script)
docker/Dockerfile
Outdated
| FROM google/cloud-sdk | ||
|
|
||
| ARG IMAGE_TAG | ||
| ENV image_tag=${IMAGE_TAG} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, given that it's the COMMIT_SHA we should call it that. Otherwise users that run
docker run gcr.io/gcp-variant-transforms/gcp-variant-transforms:1.0 echo $image_tag
will see a commit SHA1 instead of the tag they actually pulled...
| _PIPELINE_NAME = 'gcp-variant-transforms-bq-to-vcf-integration-test' | ||
| _TEST_FOLDER = 'gcp_variant_transforms/testing/integration/bq_to_vcf_tests' | ||
| _SCRIPT_PATH = '/opt/gcp_variant_transforms/bin/bq_to_vcf' | ||
| _SCRIPT_PATH = 'bq_to_vcf' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_TOOL_NAME?
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
* Update docker file: User can use docker run command to run the tool. Update the integration tests to be based on the new image.
User can use docker run command to run the tool.
Update the integration tests to be based on the new image.
Tested: integration tests.