-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: WIP experimenting with Smartview service interface #86
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Marques Johansson <[email protected]>
…ermitted with current arguments Signed-off-by: Marques Johansson <[email protected]>
Signed-off-by: Marques Johansson <[email protected]>
Signed-off-by: Marques Johansson <[email protected]>
Signed-off-by: Marques Johansson <[email protected]>
Signed-off-by: Marques Johansson <[email protected]>
id: patch | ||
if: ${{ always() && steps.fetch.conclusion == 'success' }} | ||
run: | | ||
make -f Makefile.smartview patch |
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.
We'll want to combine
before patching. We would want to be careful about the order of the source URLs being combined so that the fetched+combined spec can be patched against.
@@ -10,9 +10,12 @@ USER_AGENT=${GIT_REPO}/${PACKAGE_VERSION} | |||
|
|||
OPENAPI_IMAGE_TAG=v7.4.0 | |||
OPENAPI_IMAGE=openapitools/openapi-generator-cli:${OPENAPI_IMAGE_TAG} | |||
GOSWAGGER_IMAGE_TAG=v0.31.0 | |||
GOSWAGGER_IMAGE=ghcr.io/go-swagger/go-swagger:${GOSWAGGER_IMAGE_TAG} |
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.
go-swagger was the tool I ultimately settled on after trying a number of others. The common failing in other "combine" tools was that they left $ref: spec_1.yaml#/...
in the combined spec documents. The go-swagger flatten
feature (or attribute of combining) is what was missing in the other tools.
CRI=docker # nerdctl | ||
CRI_COMMAND_BASE=${CRI} run --rm -u ${CURRENT_UID}:${CURRENT_GID} $(DOCKER_EXTRA_ARGS) | ||
OPENAPI_GENERATOR=${CRI_COMMAND_BASE} -v $(CURDIR):/local ${OPENAPI_IMAGE} | ||
GOSWAGGER=${CRI_COMMAND_BASE} -v $(CURDIR):/local -w /local ${GOSWAGGER_IMAGE} | ||
OPENAPI_GENERATOR=${CRI_COMMAND_BASE} -v $(CURDIR):/local -w /local ${OPENAPI_IMAGE} |
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.
I added -w /local
because the argument references sometimes lead with /local
and sometimes didn't. As I moved between local tools and docker tools the /local
part of the argument list needed to be removed. Working out of /local
means we don't need to lead with /local
, but it doesn't hurt to have it.
https://developer.equinix.com/sites/default/files/smartview-v1-catalog-alert_v1_0.yaml \ | ||
https://developer.equinix.com/sites/default/files/smartview-v1-catalog-assets_v1_1.json \ | ||
https://developer.equinix.com/sites/default/files/smartview-v1-catalog-hierarchy_v1_2.json | ||
# TODO(OAS3): https://developer.equinix.com/sites/default/files/smartview-v2-catalog-system-alerts_v2_2.json |
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.
OAS3 spec was excluded.
rm -f ${SPEC_FETCHED_DIR}/* | ||
$(eval INDEX=0) | ||
for URL in ${SOURCE_URLS}; do \ | ||
${SPEC_FETCHER} \ |
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.
For whatever reason, these curl
requests are failing. I believe this was a problem we had back in early versions of metal-go that was worked-around by adding a meaningful User-Agent. No such luck this time.
-t /local/${TEMPLATE_DIR} \ | ||
-o /local/${CODE_DIR} \ | ||
--skip-validate-spec \ | ||
--openapi-normalizer FIX_DUPLICATED_OPERATIONID=true \ |
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.
FIX_DUPLICATED_OPERATIONID
may not be needed once the duplicated spec_.yaml
/ spec_2.yaml
is removed.
Signed-off-by: Marques Johansson <[email protected]>
Signed-off-by: Marques Johansson <[email protected]>
# # --merged-spec-filename /local/${MERGED_SPEC_FILE} | ||
${GOSWAGGER} mixin ${SPEC_FETCHED_DIR}/spec* --output=${SPEC_FETCHED_DIR}/merged-spec.yaml --format=yaml --keep-spec-order || true | ||
${GOSWAGGER} flatten --with-flatten=minimal ${SPEC_FETCHED_DIR}/merged-spec.yaml --output=${SPEC_FETCHED_DIR}/${SPEC_ROOT_FILE} --format=yaml | ||
# rm ${SPEC_FETCHED_DIR}/merged-spec.yaml ${SPEC_FETCHED_DIR}/spec* |
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.
We'll want to remove the source spec files when they can be fetched reliably.
Inclusion of SmartView APIs in the Equinix Go SDK repo would enable Terraform datasources and resources for interacting with these services.
This PR is intended to demonstrate how the various SmartView APIs (https://developer.equinix.com/catalog) could be merged together and packaged under a single service definition, "smartview".
Some considerations:
The generated API service is documented here: https://github.com/equinix/equinix-sdk-go/blob/a855ffb8e5f867ef1ea11e7f3c5ae79cb0f564c4/services/smartview/README.md
Note:
Browse the changes by commit to avoid the slowness of exploring all of the generating artifacts.
https://github.com/equinix/equinix-sdk-go/pull/86/commits
Ultimately this WIP may be used to recommend upstream spec definition changes, such as migration to a single OAS3 document. The combined spec file in this PR could assist in that.