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

EFSVolumeConfiguration -> efsVolumeConfiguration #2254

Merged
merged 4 commits into from
Oct 31, 2019

Conversation

sparrc
Copy link
Contributor

@sparrc sparrc commented Oct 30, 2019

Summary

For API consistency this structure has been named efsVolumeConfiguration instead of the originally-planned EFSVolumeConfiguration

Implementation details

Testing

New tests cover the changes: no

Description for the changelog

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@yumex93
Copy link
Contributor

yumex93 commented Oct 30, 2019

Can you build a test agent image, test against gamma endpoint and include the result in the pr description? Just to double check.

@@ -413,7 +413,7 @@ func (s ECRAuthData) GoString() string {
return s.String()
}

type EFSVolumeConfiguration struct {
type EfsVolumeConfiguration struct {
Copy link
Contributor

@yumex93 yumex93 Oct 30, 2019

Choose a reason for hiding this comment

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

Did you use gogenerate to generate the api.go file or the command included in the generate.go file? That should be the right way to do it. And if you did that, the struct name should still beEFSVolumeConfiguration.

Copy link
Member

@fierlion fierlion left a comment

Choose a reason for hiding this comment

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

Approved if I can see some test output, namely the results of all the fun new tests you added:

TestMarshalTaskVolumesEFS
TestUnmarshalTaskVolumesEFS
TestPostUnmarshalTaskWithEFSVolumes

@sparrc
Copy link
Contributor Author

sparrc commented Oct 31, 2019

Approved if I can see some test output, namely the results of all the fun new tests you added:

TestMarshalTaskVolumesEFS
TestUnmarshalTaskVolumesEFS
TestPostUnmarshalTaskWithEFSVolumes

sure....although it's nothing too exciting:

% /usr/local/go/bin/go test -count=1 -v -cover -tags unit -timeout=60s -run=arshalTaskVolumes ./agent/api/task/...
=== RUN TestMarshalTaskVolumesEFS
--- PASS: TestMarshalTaskVolumesEFS (0.00s)
=== RUN TestUnmarshalTaskVolumesEFS
--- PASS: TestUnmarshalTaskVolumesEFS (0.00s)
=== RUN TestMarshalUnmarshalTaskVolumes
--- PASS: TestMarshalUnmarshalTaskVolumes (0.00s)
PASS

@sparrc sparrc merged commit 0dcbfd6 into aws:feature/efs-volume-config Oct 31, 2019
sparrc added a commit that referenced this pull request Nov 5, 2019
* EFSVolumeConfiguration -> efsVolumeConfiguration

* Fix FileSystemId and write json marshalling unit tests

* unit test PostUnmarshalTask efsVolumeConfiguration behavior

* windows unit test fix
sparrc added a commit that referenced this pull request Dec 4, 2019
* EFSVolumeConfiguration -> efsVolumeConfiguration

* Fix FileSystemId and write json marshalling unit tests

* unit test PostUnmarshalTask efsVolumeConfiguration behavior

* windows unit test fix
sparrc added a commit that referenced this pull request Dec 5, 2019
* EFSVolumeConfiguration -> efsVolumeConfiguration

* Fix FileSystemId and write json marshalling unit tests

* unit test PostUnmarshalTask efsVolumeConfiguration behavior

* windows unit test fix
sparrc added a commit that referenced this pull request Dec 6, 2019
* Added EFSVolumeConfiguration (#2234)

* Added EFSVolumeConfiguration models

* Translate EFS volumes from ACS to Docker volume type

* fix gocyclo failure

* code review comments

* remove readonly config

* remove readonly options from code

* code review comments

* code review

* naming is hard

* Add efs capability (#2248)

* EFS functional testing (#2247)

* Add efs client to Gopkg.*

* Add efs client to vendor directory

* EFS functional test

* Reuse EFS filesystem and mount target(s)

* review fixups

* more code review fixups

* EFSVolumeConfiguration -> efsVolumeConfiguration (#2254)

* EFSVolumeConfiguration -> efsVolumeConfiguration

* Fix FileSystemId and write json marshalling unit tests

* unit test PostUnmarshalTask efsVolumeConfiguration behavior

* windows unit test fix

* Update ECS client and task defs (#2256)

* add memoryUnbounded to task volume marshal unit tests

* rebase mistake
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.

3 participants