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

Drop support for VCD 9.5 (bump API version to 32.0) #330

Merged
merged 11 commits into from
Aug 25, 2020

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented Aug 18, 2020

This PR does one thing - it drops support for VCD 9.5 by bumping default API version to 32.0. During PR reviews the following things were changed:

  • Drop support for VCD 9.5 (bump default API version to 32.0)
  • CheckOpenApiEndpointCompatibility() returns system default API version when it is higher than minimum required
  • Remove code for handling API versions pre 32.0
  • Deprecated vdc.UploadMediaImage because it does not work anymore with API v32.0

Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

LGTM :) Please also raise a PR to drop support in Terraform provider (e.g. updating the front page of the docs).

Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

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

Please remove things like this also:

adminOrg.client.GetSpecificApiVersionOnCondition(">= 32.0", "32.0"))

@Didainius
Copy link
Collaborator Author

Please remove things like this also:

adminOrg.client.GetSpecificApiVersionOnCondition(">= 32.0", "32.0"))

True. I wiped some stuff.

@vbauzys
Copy link
Contributor

vbauzys commented Aug 18, 2020

Please remove things like this also:
adminOrg.client.GetSpecificApiVersionOnCondition(">= 32.0", "32.0"))

True. I wiped some stuff.

nice work.

@vbauzys
Copy link
Contributor

vbauzys commented Aug 19, 2020

I think one more change is needed - merge from master and upgrade this
types.OpenApiEndpointRoles: "31.0",

@Didainius
Copy link
Collaborator Author

I think one more change is needed - merge from master and upgrade this
types.OpenApiEndpointRoles: "31.0",

Well spotted and I think there is room for improvement in checkOpenApiEndpointCompatibility. Now it returns the version in the map, but if the default version is higher than "version introduced" it should default to the default version we use.

@Didainius
Copy link
Collaborator Author

I think one more change is needed - merge from master and upgrade this
types.OpenApiEndpointRoles: "31.0",

Well spotted and I think there is room for improvement in checkOpenApiEndpointCompatibility. Now it returns the version in the map, but if the default version is higher than "version introduced" it should default to the default version we use.

Do have a look at commit 7e5e2e0. This improved on behavior of OpenAPI function checkOpenApiEndpointCompatibility. It will remove need of manual labour to bump versions in endpointMinApiVersions when we bump default system API version. cc @dataclouder , @lvirbalas

@Didainius
Copy link
Collaborator Author

Reran make testcatalog on VCD 9.7 and 10 after latest merge. It succeeded including the new config.

Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

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

Validate with 10.1. Looks ok.

One thing:

now createVdcV97 and CreateVdc duplicates one another as other vDC functions

@Didainius
Copy link
Collaborator Author

Validate with 10.1. Looks ok.

One thing:

now createVdcV97 and CreateVdc duplicates one another as other vDC functions

Not exactly duplicates. I removed createVdc, updateVdc etc, old functions.
The public wrapper functions like CreateVdc still leave the capability for dynamic version handling if have to introduce it back.

@vbauzys
Copy link
Contributor

vbauzys commented Aug 24, 2020

Validate with 10.1. Looks ok.
One thing:
now createVdcV97 and CreateVdc duplicates one another as other vDC functions

Not exactly duplicates. I removed createVdc, updateVdc etc, old functions.
The public wrapper functions like CreateVdc still leave the capability for dynamic version handling if have to introduce it back.

Missed that function is deprecated. So we will remove that and current will be latest.

@Didainius Didainius merged commit 8886b5b into vmware:master Aug 25, 2020
@Didainius Didainius deleted the bump_api_version branch August 25, 2020 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants