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

Subpath Mounts on Datasets #101

Merged
merged 12 commits into from Jun 26, 2019

Conversation

Projects
None yet
3 participants
@Amusement
Copy link
Contributor

commented Jun 17, 2019

@splittingfield
Copy link
Member

left a comment

Leaving comments before real engineers comment. I am not confident that the optional part of the TODO is actually handled.

@@ -108,6 +108,9 @@ type DatasetMount struct {
// (required) Name or Unique ID of a dataset to mount.
Dataset string `json:"dataset" yaml:"datasetId"`

// (optional) Path within the dataset to mount for this experiment container.

This comment has been minimized.

Copy link
@splittingfield

splittingfield Jun 17, 2019

Member

I believe if this is optional that the type needs to be *string.

This comment has been minimized.

Copy link
@splittingfield

splittingfield Jun 17, 2019

Member

or an omitEmpty. I always forget with go.

This comment has been minimized.

Copy link
@viveklak

viveklak Jun 21, 2019

Contributor

For future reference, definitely prefer using omitempty over pointers.

Amusement and others added some commits Jun 17, 2019

@Amusement Amusement merged commit 1d64e87 into master Jun 26, 2019

1 check passed

ci/circleci: build Your tests passed on CircleCI!
Details

@Amusement Amusement deleted the naph/subpath-mounts branch Jun 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.