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

Updating the latest public fabric v4 [4.6] API specifications #18

Merged
merged 7 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ SPEC_FETCHED_PATCHES=patches/spec.fetched.json
# OpenAPI codegen container
##
CRI:=docker # nerdctl
OPENAPI_CODEGEN_TAG=latest
OPENAPI_CODEGEN_TAG=v6.4.0
OPENAPI_CODEGEN_IMAGE=openapitools/openapi-generator-cli:${OPENAPI_CODEGEN_TAG}
DOCKER_OPENAPI=${CRI} run --rm -u ${CURRENT_UID}:${CURRENT_GID} -v $(CURDIR):/local ${OPENAPI_CODEGEN_IMAGE}

Expand All @@ -46,8 +46,10 @@ docker_generate:
# Utility functions
##

# fetch:
# curl ${OPENAPI_URL} | jq . > ${SPEC_FETCHED_FILE}
# Fetch any public available version of Fabric V4 API specification. Send the URL to the specification as input argument
# Example: make fetch OPENAPI_URL=https://app.swaggerhub.com/apiproxy/registry/equinix-api/fabric/4.6
fetch:
curl ${OPENAPI_URL} | jq . > ${SPEC_FETCHED_FILE}
Copy link
Member

Choose a reason for hiding this comment

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

Let's define this in the Makefile (around line 14) similar to https://github.com/equinix-labs/metal-java/blob/main/Makefile#L10-L11

OPENAPI_URL=https://app.swaggerhub.com/apiproxy/registry/equinix-api/fabric/4.6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


clean:
rm -rf ${OPENAPI_GENERATED_CLIENT}
Expand Down

This file was deleted.

This file was deleted.

14 changes: 0 additions & 14 deletions patches/spec.fetched.json/03-remove-project-id-as-required.patch

This file was deleted.

6 changes: 1 addition & 5 deletions patches/spec.fetched.json/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1 @@
## Patches requirement details :

01 : Invalid usage of type attribute in model.

02 : Usage of discriminator with referenced ENUM type produces invalid java bindings (fix needs to be in openapi generator's java template).
## Patches requirement details :
2 changes: 1 addition & 1 deletion spec/oas3.fabric.config.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"groupId": "com.equinix",
"artifactId": "equinix-openapi-fabric",
"artifactVersion": "1.0.0",
"artifactVersion": "4.6",
Copy link
Member

Choose a reason for hiding this comment

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

This would be a move away from semantic versioning. Does the upstream API have an API patch revision? If it does then we should represent that full version in this project. This repo would then need to represent the fabric-java revision or build as a suffix to the upstream semver version: v4.6.0-1

Alternatively, if the upstream API is only expressed as x.y, we could use .z to represent the fabric-java release of that artifact release.

In either of these scenarios, breaking changes in the SDK, introduced through codegen changes rather than upstream API spec changes, will not be reflected through semver.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Let`s use the versioning of the artifact to have x.y.z where z represents the patch version.
Change done.

Copy link
Member

@displague displague Mar 31, 2023

Choose a reason for hiding this comment

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

We had some followup conversation in Slack based on how this field is used, how the Equinix Fabric API is versioned, and how changes to this SDK may be versioned independently of the Fabric API.

artifactVersion | artifact version in generated pom.xml. This also becomes part of the generated library's filename

The version that we'll use in this project will start with v0.1.0 as a semver release and we can update that version according to ANY change in this SDK (Makefile, patches, docs, upstream API). By keeping the revision under v1, we can make large changes to the SDK without disturbing user expectation (v0 implies all changes are possible).

We will include the Equinix Fabric API version in the package description and we can introduce SDK version updates to newer versions of the Fabric API, for example, a hypothetical release history:

API Version SDK Release Latest Commit Message
4.6 v0.1.0 Initial SDK release for Fabric 4.6
4.6 v0.2.0 SDK models moved to com.equinix.more.fabric from com.equinix.openapi.fabric
4.6 v0.2.1 makefile fixed to correct errors in the documentation
4.7 v0.3.0 Fabric API spec updated to 4.7 and code regenerated

We can always move up to a v4.6+ semver for the SDK later if we feel that is a more appropriate pattern. If we choose to follow that pattern, we may use v4.6.0 (0 as the patch release because the upstream API does not publish a patch release number) with a suffix such as -build.{sdk semver}.

Other alternatives considered were to include a <properties><version> element in the POM.

"library": "okhttp-gson",
"invokerPackage": "com.equinix.openapi.fabric",
"modelPackage": "com.equinix.openapi.fabric.v4.model",
Expand Down
Loading