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

enable query orgs #612

Closed
wants to merge 7 commits into from
Closed

enable query orgs #612

wants to merge 7 commits into from

Conversation

medea61
Copy link

@medea61 medea61 commented Aug 25, 2023

Description

Related issue: #611

  • Enable to query for organisations

  • Adds OrgRecord to QueryResultRecordsType and QueryResultOrgRecordType

@vmwclabot
Copy link
Member

@medea61, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@vmwclabot
Copy link
Member

@medea61, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed.

@dataclouder
Copy link
Collaborator

Thanks for your contribution.
We don't have a query for organizations because our software clients have not required it yet.
That said, we can add the feature, but we will do so only if it is accompanied by proper testing.
Look at Test_QueryProviderVdcByName for an example of the test we would like to see .
A Query element per se is not useful: it should have at least three auxiliary functions:

  • QueryAllOrgs (retrieve all organizations without filtering)
  • QueryOrgByName (Retrieve a specific org by name)
  • QueryOrgByID (Retrieve a specific Org by its ID)

@vmwclabot
Copy link
Member

@medea61, VMware has approved your signed contributor license agreement.

@dataclouder dataclouder self-assigned this Aug 25, 2023
@dataclouder
Copy link
Collaborator

Please make sure your code compiles before asking for review

go test -tags functional -check.f Test_QueryOrgByName -check.vv -timeout 0
# github.com/vmware/go-vcloud-director/v2/govcd [github.com/vmware/go-vcloud-director/v2/govcd.test]
./system_test.go:764:24: vcd.config.VCD.Org.Name undefined (type string has no field or method Name)
./system_test.go:769:26: vcd.client.QueryOrgByName undefined (type *VCDClient has no field or method QueryOrgByName)
./system_test.go:769:52: vcd.config.Org undefined (type TestConfig has no field or method Org)
./system_test.go:775:25: vcd.config.VCD.Org.Name undefined (type string has no field or method Name)
./system_test.go:794:24: vcd.config.VCD.Org.Id undefined (type string has no field or method Id)
./system_test.go:799:26: vcd.client.QueryOrgById undefined (type *VCDClient has no field or method QueryOrgById)
./system_test.go:799:50: vcd.config.Org undefined (type TestConfig has no field or method Org)
./system_test.go:805:25: vcd.config.VCD.Org.Name undefined (type string has no field or method Name)

@dataclouder dataclouder marked this pull request as draft August 25, 2023 14:41
@medea61
Copy link
Author

medea61 commented Aug 25, 2023

Please make sure your code compiles before asking for review

Tests compile now. Can't run the tests locally at the moment as I have only prod deployments. 🙈

@medea61 medea61 marked this pull request as ready for review August 25, 2023 16:05
types/v56/types.go Outdated Show resolved Hide resolved
govcd/system_test.go Show resolved Hide resolved
govcd/org.go Outdated Show resolved Hide resolved
@@ -212,6 +215,7 @@ func (client *Client) cumulativeQueryWithHeaders(queryType string, params, notEn
types.QtResourcePool,
types.QtNetworkPool,
types.QtProviderVdcStorageProfile,
types.QtOrg,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you have added some of the pieces needed to run Org queries through cumulative query, you should also add this to addResults

	case types.QtOrg:
		cumulativeResults.Results.OrgRecord = append(cumulativeResults.Results.OrgRecord, newResults.Results.OrgRecord...)
		size = len(newResults.Results.OrgRecord)

@lvirbalas
Copy link
Collaborator

@medea61 this PR is very close to approval. Would you have a chance to implement the remaining asks?

@dataclouder
Copy link
Collaborator

Thanks for your contribution. This code has been merged with PR #669

@dataclouder dataclouder mentioned this pull request Jun 13, 2024
10 tasks
@dataclouder
Copy link
Collaborator

Added to main through PR #669

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.

None yet

4 participants