sharedfilesystems: Add CreateShareFromSnapshotSupport field to Share struct

[gman0]

This PR adds CreateShareFromSnapshotSupport field to Share struct. It maps to create_share_from_snapshot_support in response.

For #2190

Links to the line numbers/files in the OpenStack source code that support the
code in this PR:

https://docs.openstack.org/api-ref/shared-file-system/?expanded=show-share-details-detail#id27

[coveralls]

Coverage remained the same at 79.872% when pulling bebec5c on gman0:manila-share-field-sharefromsnap into 5a7ac65 on gophercloud:master.

[theopenlab-ci]

Build succeeded.

• gophercloud-unittest : SUCCESS in 4m 37s
• gophercloud-acceptance-test : SUCCESS in 3h 32m 36s
• gophercloud-acceptance-test-ironic : SUCCESS in 19m 51s

[gman0]

PTAL @gouthampacha @jtopjian

[jtopjian]

@gman0 Thanks for submitting this.

Can you provide the Manila API code reference that defines this field? Also, are there any unit tests or acceptance tests that can be included in this PR?

[gouthampacha]

@gman0 Thanks for submitting this.

Can you provide the Manila API code reference that defines this field? Also, are there any unit tests or acceptance tests that can be included in this PR?

I suppose you want the API ref in the PR commit message, @jtopjian ?
https://docs.openstack.org/api-ref/shared-file-system/?expanded=show-share-details-detail#show-share-details

As far as tests are concerned, looks like we could update the fixtures here:

gophercloud/openstack/sharedfilesystems/v2/shares/testing/fixtures.go

Line 26 in 5a7ac65

"share": {

gophercloud/openstack/sharedfilesystems/v2/shares/testing/fixtures.go

Line 101 in 5a7ac65

var updateResponse = `

gophercloud/openstack/sharedfilesystems/v2/shares/testing/fixtures.go

Line 157 in 5a7ac65

var getResponse = `{

gophercloud/openstack/sharedfilesystems/v2/shares/testing/fixtures.go

Line 209 in 5a7ac65

var listDetailResponse = `{

and the assertions here: https://github.com/gophercloud/gophercloud/blob/master/openstack/sharedfilesystems/v2/shares/testing/request_test.go

This seems like a simple case where a missing parameter's being added, but the API ref should contain the right response objects to copy over..

[jtopjian]

@gman0

I suppose you want the API ref in the PR commit message

I was looking for something along the lines of this: https://github.com/openstack/manila/blob/a3aaea91494665a25bdccebf69d9e85e8475983d/manila/api/views/shares.py#L147-L151

If I understand the Manila code correctly, this block of code amends the view/result with the create_share_from_snapshot_support field, but I could be wrong. Step 3 of the contributor guide explains why this is important.

As far as tests are concerned, looks like we could update the fixtures here:

Yeah, updating fixtures (both the JSON responses and mock structs) should be fine. The important piece here is to keep the fixtures up-to-date in case missing fields cause an issue in the future. Unit tests currently pass because this is a boolean field, which defaults to false, which is the default value when a struct field is omitted, so it's more of a coincidence that the tests are passing.

Let me know if this helps or if you have any questions.

[gouthampacha]

@gman0

I suppose you want the API ref in the PR commit message

I was looking for something along the lines of this: https://github.com/openstack/manila/blob/a3aaea91494665a25bdccebf69d9e85e8475983d/manila/api/views/shares.py#L147-L151

If I understand the Manila code correctly, this block of code amends the view/result with the create_share_from_snapshot_support field, but I could be wrong. Step 3 of the contributor guide explains why this is important.

Totally, thank you for the link to the code-hunting doc. That explains what you were looking for. While it's true that this API can be extended/customized, the intent here is to expose the share object that will be common across all conformant openstack clouds.

As far as tests are concerned, looks like we could update the fixtures here:

Yeah, updating fixtures (both the JSON responses and mock structs) should be fine. The important piece here is to keep the fixtures up-to-date in case missing fields cause an issue in the future. Unit tests currently pass because this is a boolean field, which defaults to false, which is the default value when a struct field is omitted, so it's more of a coincidence that the tests are passing.

+1 Thanks for your response

Let me know if this helps or if you have any questions.

[gman0]

Apologies, I was busy yesterday... Thank you both for suggestions around tests and finding the relevant places in src! I'll update the fixtures and their assertions.

[theopenlab-ci]

Build succeeded.

• gophercloud-unittest : SUCCESS in 4m 35s
• gophercloud-acceptance-test : SUCCESS in 3h 41m 29s
• gophercloud-acceptance-test-ironic : SUCCESS in 16m 31s

[jtopjian]

@gouthampacha Sorry. I @'d the wrong person yesterday.

Thank you both. This looks good to me!

[jtopjian]

LGTM - thank you!

[gman0]

Thank you!

[gman0]

Hi @jtopjian just a quick question: from the looks of things it seems gophercloud releases are cca 1 month apart. Are you planning to make July's release still, or is v0.19.0 coming out later? We'd like to use this feature already, so I'd like to know whether we should wait a bit more so we can update our dependencies then, or we should just pin our go.mod to this commit now.

[jtopjian]

@gman0 I've just created v0.19.0. Let me know if you need anything else 🙂

[gman0]

@jtopjian that's excellent news, thank you!