Refactored Get* methods 1 #217
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
July 16, 2019 15:58
* Add function getUuidFromHref * Add function getOrgHREFById, to retrieve an HREF from an Org/AdminOrg ID * Add method vcdClient.ReadOrgByName * Add method vcdClient.ReadOrgById * Add method vcdClient.ReadOrgByNameOrId * Add method vcdClient.ReadOrg (using a struct) * Add method vcdClient.ReadAdminOrgByName * Add method vcdClient.ReadAdminOrgById * Add method vcdClient.ReadAdminOrgByNameOrId * Add method vcdClient.ReadAdminOrg (using a struct) * Add Fix for values in OrgGeneralSettings, which are ignored unless some fields are always filled. * Add test for getUuidFromHref * Add tests for all Read* methods * Add more tests for Org creation
* Deprecate GetOrgByName, GetAdminOrgByName * Replace all calls to the above function with the new Read* methods
The Update method was not updating the Org description
added 7 commits
July 19, 2019 16:02
* Remove unused methods GetOrg and GetAdminOrg * Remove corresponding tests * Standardize deprecation messages * Add Changelog entries * Add CODING_PRINCIPLES.md
Didainius
reviewed
Jul 30, 2019
There was a problem hiding this comment.
A lot of good things:
- Thanks for the "it's" fixes...
- +1 for fixed deprecation messages
- Nice on the coding guidelines.
One spot/question - in many places I see:
check.Assert(org, NotNil)
check.Assert(err, IsNil)
Wouldn't it be more informative to swap them so that check.Assert(err, IsNil) is the first check. If it does not pass the dumped err should be more informative than a simply nil struct pointer of some type.
vbauzys
suggested changes
Jul 30, 2019
added 4 commits
July 31, 2019 13:12
Fetch* functions were renamed in this PR, but they should have been deprecated instead. They are now reinstated as wrappers to the intended replacements
Make the flow easier to read and more robust
Make the functions that retrieve by name or ID easier to read and more robust against corner cases where an error may occur in the first search that is not a ErrorEntityNotFound, and would possibly return stale results.
This move allows running org tests by tag.
Didainius
reviewed
Aug 1, 2019
Didainius
reviewed
Aug 1, 2019
Didainius
approved these changes
Aug 1, 2019
vbauzys
reviewed
Aug 1, 2019
| @@ -337,6 +365,37 @@ func getOrgHREF(vcdClient *VCDClient, orgName string) (string, error) { | |||
| return "", fmt.Errorf("couldn't find org with name: %s. Please check Org name as it is case sensitive", orgName) | |||
| } | |||
|
|
|||
| // Returns the HREF of the org from the org ID | |||
| func getOrgHREFById(vcdClient *VCDClient, orgId string) (string, error) { | |||
| orgListHREF := vcdClient.Client.VCDHREF | |||
There was a problem hiding this comment.
Maybe it's possible to Query using href/id in filter? this would make function very simple
There was a problem hiding this comment.
Query struct doesn't have Organization info.
There was a problem hiding this comment.
Yes, It would be need to add.
vbauzys
approved these changes
Aug 1, 2019
lvirbalas
approved these changes
Aug 2, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first batch of changes to support the creation of data sources in terraform-provider-vcd.
Changes:
Note: when a function was returning
Organd other entities, I have only changed the return type forOrg(from structure to pointer), leaving the refactoring of the other return types when we tackle the other entities.