-
Notifications
You must be signed in to change notification settings - Fork 104
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
Use equivalent item values to find existing property GUIDs #503
Use equivalent item values to find existing property GUIDs #503
Conversation
We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story. The labels on this github issue will be updated when the story is started. |
@@ -9,17 +10,25 @@ import ( | |||
) | |||
|
|||
var _ = Describe("ResponsePropertyCollection", func() { | |||
unmarshalJSON := func(json string) interface{} { | |||
unmarshalJSONLikeApiGetStagedProductProperties := func(json string) interface{} { |
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.
In the tests I needed to unmarshal JSON using both the json
and yaml
packages in order to faithfully simulate the behaviour of om/api/staged_products_service.go
Out of interest why does Api.GetStagedProductProperties
use yaml.Unmarshal()
rather than json.Unmarshal()
?
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.
No particular reason. We inherited this codebase from the releng team, and haven't made everything consistent yet
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.
Good to know.
My 2c - we should switch to json.Unmarshal
; since it produces more usable map[string]interface{}
interfaces.
… the same collection
…existing item data
This looks good to us. Stylistically we don't see any problems. We're going to merge in and test. Thanks for the PR! |
Related to #502, this addition extends the decorating collection with guid logic to associate collection item guids with items that have equivalent item.
For example (see test); if the following item exists in an OpsMgr Property Collection:
And an operator uses
om configure-product
to "update" that property with no changes (as might be the case in a GitOps workflow):om configure-product
will decorate the updated property with theguid
of the existing equivalent item (ignoring key order); which would register (correctly) as ano-op
change by theom bosh-diff
functionality; preserving the existing itemguid
This partially addresses #207
Known issues