Accommodate non-Amazon data center info metadata #44
Conversation
| Metadata metadataMap `xml:"metadata" json:"metadata"` | ||
| } | ||
|
|
||
| func bindValue(dst *string, m map[string]string, k string) bool { |
There was a problem hiding this comment.
Should m here be src instead like at line 236 below?
There was a problem hiding this comment.
Up to you. You are an artist in this PR :)
| } | ||
| } | ||
|
|
||
| return e.EncodeToken(start.End()) |
There was a problem hiding this comment.
Should we call Flush() here?
https://golang.org/pkg/encoding/xml/#Encoder.EncodeToken
There was a problem hiding this comment.
The documentation mentions creating an Encoder:
Callers that create an
Encoderand then invokeEncodeTokendirectly, without usingEncodeorEncodeElement, need to callFlushwhen finished to ensure that the XML is written to the underlying writer.
Here, I didn't create the Encoder; it's created implicitly over in fargo.marshal in file net.go. Note that xml.Marshal creates an Encoder and calls Encode on it, which in turn flushes when it's done writing. Hence I don't think that calling Flush explicitly here is necessary.
|
I had a few minor comments, other then that it looks good 👍 Thank you for great unit tests! |
The existing "Metadata" field in the DataCenterInfo struct conveys details specific to instances running within one of Amazon's data centers. For instances running in other types of data centers, introduce a sibling "AlternateMetadata" field, sending and populating it only when the DataCenterInfo's "Name" field is "Amazon". Retaining the original "Metadata" field maintains compatibility with existing callers, unless they had populated that field for data centers with names other than "Amazon".
seh
force-pushed
the
allow-other-dc-metadata
branch
from
6de7bd6 to
bb10d2c
Compare
|
PTAL. |
|
Changes are good to go. I need to test them and will release that. Thank you very much @seh |
|
Fantastic. Thank you. |
These proposed changes address #42, including tests to demonstrate the special marshaling policy for the
fargo.DataCenterInfostruct.Let me know if you think we should define a method on
DataCenterInfolikeAddMetadataEntry(key, value string)that would create the map for theAlternateMetadatafield if it's still nil.