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

importer-rest-api-specs: temporary workaround for #3325 to avoid flapping diff in API definitions #4434

Merged

Conversation

manicminer
Copy link
Contributor

@manicminer manicminer commented Sep 23, 2024

Temporary workaround for #3325

@manicminer manicminer added bug Something isn't working tool/importer-rest-api-specs Swagger Data Importer issues labels Sep 23, 2024
Copy link
Member

@stephybun stephybun left a comment

Choose a reason for hiding this comment

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

Thanks @manicminer LGTM 🥟

ReadOnly: false,
Sensitive: false,
JsonName: "notify-keyspace-events",
//Description: "The KeySpace Events which should be monitored.", // TODO: avoid flapping diff in api definitions, see https://github.com/hashicorp/pandora/issues/3325
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this needs to be commented out since it's a workaround and we're adding this data instead of attempting to parse it from the spec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I've undone that 👍

@manicminer manicminer force-pushed the bugfix/workaround-for-inconsistent-model-field-description branch from d1ccdca to 128cd0f Compare September 23, 2024 09:49
@manicminer manicminer merged commit 345b393 into main Sep 23, 2024
3 checks passed
@manicminer manicminer deleted the bugfix/workaround-for-inconsistent-model-field-description branch September 23, 2024 10:44
stephybun added a commit that referenced this pull request Oct 9, 2024
* `tools/importer-rest-api-specs`: refactoring the `commonschema` Matchers out into the new structure

* `tools/importer-rest-api-specs`: refactoring the Constant package

* `tools/importer-rest-api-specs`: porting the Network workaround into a Data Workaround

* `tools/importer-rest-api-specs`: porting the `dataworkarounds` package over to using the SDK types/new path

This is a copy and patch up, since I want to leave the existing code path as-is for the moment, to break it apart more effectively

* `tools/importer-rest-api-specs`: scaffolding out the logic to parse a Service/APIVersion

* `tools/importer-rest-api-specs`: porting over the "remove unused items" logic

* `tools/importer-rest-api-specs`: adding a shim to the updated `dataworkarounds` package

* dependencies: updating to `v0.69.0` of `github.com/hashicorp/go-azure-helpers`

* `tools/importer-rest-api-specs`: refactoring the `normalization` package into the new structure

* `tools/importer-rest-api-specs`: moving all of the `cleanup` items into the new package

This wants refactoring, but moving it over allows cleaning the rest up

* `tools/importer-rest-api-specs`: adding inner most sdk operation object definition

* `tools/importer-rest-api-specs`: moving the `commonids` into a subpackage

There's a bunch of these, and whilst these'll get replaced in time with #4017
but for now this cleans this up

* `tools/data-api-sdk`: removing an empty todo file

* `tools/importer-rest-api-specs`: refactoring more logic out to the new package

The `parser` package itself needs cleaning, but there's a few things to do first

* `tools/importer-rest-api-specs`: moving another `ignore` function

* WIP

* tools/importer-rest-api-specs: reimplementing #4290 atop the rebase

The rebase is messed up, but it's a minor patch so should be fine

* `tools/importer-rest-api-specs`: adding a placeholder for parsing an APIResource

* `tools/importer-rest-api-specs`: refactoring the parser into smaller packages

This leaves the old parser logic in place for now, which'll be cleaned up in the next commit

* `tools/importer-rest-api-specs`: fixing an issue where the `validate` command didn't pass the API Definitions Directory correctly

* `tools/importer-rest-api-specs`: title-casing the ReferenceName - which ensures that when we compare the ObjectDefinition from a previously parsed/currently parsed dataset we have matching values

These are normalised post-parsing, so when comparing against the old data set we need to use TitleCase too, so we might as well do this here

* `tools/importer-rest-api-specs`: ensuring that TypeSpec examples are filtered out

* `tools/importer-rest-api-specs`: trimming `*.json` from the filename to build the api resource name

* `tools/importer-rest-api-specs`: removing (most) of the old code path

* `tools/data-api-sdk`: making IsCommonType required so this is more explicit

* `tools/importer-rest-api-specs`: fixing the validation

* `tools/importer-rest-api-specs`: re-enabling most of the tests

* `tools/importer-rest-api-specs`: consolidating/porting over the data tests

These ensure that the Configs can be parsed and that the Data from them can be loaded

* `tools/importer-rest-api-specs`: fixing the test format

* tools/importer-rest-api-specs: skipping the local test

* `tools/importer-rest-api-specs`: fixing the non-discriminator tests

* `tools/importer-rest-api-specs`: updating/reworking the supplementary data test

@manicminer is going to work through fixing this one up, this requires parsing the data identifying any parent types and patching any disciminated implementations across

* `importer-rest-api-specs`: fix up discriminator handling for datafactory

* `importer-rest-api-specs`: shelve SupplementaryData support for now, since it doesn't seem to be required for DataFactory models as they use external Swagger Refs.

* `importer-rest-api-specs`: implement `-services` argument

* `importer-rest-api-specs`: operate on maps instead of passing them around

* `importer-rest-api-specs`: normalize swagger tag names to prevent duplicates (avoid parsing the same tag twice)

* `importer-rest-api-specs`: normalize API resources after parsing all of them

* `importer-rest-api-specs`: ensure new APIResource is populated

* `importer-rest-api-specs`: maps are pointers

* `importer-rest-api-specs`: dereference more maps

* `importer-rest-api-specs`: remove use of strings.Title, dereference more maps

* limit scope of SDK generation test to the SDK being tested

* `importer-rest-api-specs`: two loops become one

* `importer-rest-api-specs`: workaround inconsistent casing of "VirtualWANs"

* Revert "`importer-rest-api-specs`: two loops become one"

This reverts commit 31b45ca.

* `importer-rest-api-specs`: normalize "attachednetworks" segment of resource IDs

* Revert "`tools/data-api-sdk`: making IsCommonType required so this is more explicit"

This reverts commit 71a166f.

* importer-rest-api-specs: use updated forks for `go-openapi/analysis`, `go-openapi/jsonreference`, and `go-openapi/spec`

* importer-rest-api-specs: remove Supplementary Data parser as is not used/needed

* importer-rest-api-specs: rework the swagger spec parser and manually resolve all remote references

* importer-rest-api-specs: fix format string, ensure model names are URL-decoded when parsing from the Ref URL fragment

* importer-rest-api-specs: updated tests for discriminated models to expect full ancestry

* importer-rest-api-specs: remove vestiges of supplementary data spike

* importer-rest-api-specs: do not set the description, see #4434

* importer-rest-api-specs: hack for conflicting constant names in the same set of resources

* importer-rest-api-specs: disable check for non-default Resource Provider when parsing operations, as this was never utilised and leads to skipping resources/operations

* upper case URL in the go sdk

---------

Co-authored-by: Tom Bamford <tom@bamford.io>
Co-authored-by: Steph <steph@hashicorp.com>
zaowang-ms pushed a commit to zaowang-ms/pandora that referenced this pull request Oct 22, 2024
…icorp#4307)

* `tools/importer-rest-api-specs`: refactoring the `commonschema` Matchers out into the new structure

* `tools/importer-rest-api-specs`: refactoring the Constant package

* `tools/importer-rest-api-specs`: porting the Network workaround into a Data Workaround

* `tools/importer-rest-api-specs`: porting the `dataworkarounds` package over to using the SDK types/new path

This is a copy and patch up, since I want to leave the existing code path as-is for the moment, to break it apart more effectively

* `tools/importer-rest-api-specs`: scaffolding out the logic to parse a Service/APIVersion

* `tools/importer-rest-api-specs`: porting over the "remove unused items" logic

* `tools/importer-rest-api-specs`: adding a shim to the updated `dataworkarounds` package

* dependencies: updating to `v0.69.0` of `github.com/hashicorp/go-azure-helpers`

* `tools/importer-rest-api-specs`: refactoring the `normalization` package into the new structure

* `tools/importer-rest-api-specs`: moving all of the `cleanup` items into the new package

This wants refactoring, but moving it over allows cleaning the rest up

* `tools/importer-rest-api-specs`: adding inner most sdk operation object definition

* `tools/importer-rest-api-specs`: moving the `commonids` into a subpackage

There's a bunch of these, and whilst these'll get replaced in time with hashicorp#4017
but for now this cleans this up

* `tools/data-api-sdk`: removing an empty todo file

* `tools/importer-rest-api-specs`: refactoring more logic out to the new package

The `parser` package itself needs cleaning, but there's a few things to do first

* `tools/importer-rest-api-specs`: moving another `ignore` function

* WIP

* tools/importer-rest-api-specs: reimplementing hashicorp#4290 atop the rebase

The rebase is messed up, but it's a minor patch so should be fine

* `tools/importer-rest-api-specs`: adding a placeholder for parsing an APIResource

* `tools/importer-rest-api-specs`: refactoring the parser into smaller packages

This leaves the old parser logic in place for now, which'll be cleaned up in the next commit

* `tools/importer-rest-api-specs`: fixing an issue where the `validate` command didn't pass the API Definitions Directory correctly

* `tools/importer-rest-api-specs`: title-casing the ReferenceName - which ensures that when we compare the ObjectDefinition from a previously parsed/currently parsed dataset we have matching values

These are normalised post-parsing, so when comparing against the old data set we need to use TitleCase too, so we might as well do this here

* `tools/importer-rest-api-specs`: ensuring that TypeSpec examples are filtered out

* `tools/importer-rest-api-specs`: trimming `*.json` from the filename to build the api resource name

* `tools/importer-rest-api-specs`: removing (most) of the old code path

* `tools/data-api-sdk`: making IsCommonType required so this is more explicit

* `tools/importer-rest-api-specs`: fixing the validation

* `tools/importer-rest-api-specs`: re-enabling most of the tests

* `tools/importer-rest-api-specs`: consolidating/porting over the data tests

These ensure that the Configs can be parsed and that the Data from them can be loaded

* `tools/importer-rest-api-specs`: fixing the test format

* tools/importer-rest-api-specs: skipping the local test

* `tools/importer-rest-api-specs`: fixing the non-discriminator tests

* `tools/importer-rest-api-specs`: updating/reworking the supplementary data test

@manicminer is going to work through fixing this one up, this requires parsing the data identifying any parent types and patching any disciminated implementations across

* `importer-rest-api-specs`: fix up discriminator handling for datafactory

* `importer-rest-api-specs`: shelve SupplementaryData support for now, since it doesn't seem to be required for DataFactory models as they use external Swagger Refs.

* `importer-rest-api-specs`: implement `-services` argument

* `importer-rest-api-specs`: operate on maps instead of passing them around

* `importer-rest-api-specs`: normalize swagger tag names to prevent duplicates (avoid parsing the same tag twice)

* `importer-rest-api-specs`: normalize API resources after parsing all of them

* `importer-rest-api-specs`: ensure new APIResource is populated

* `importer-rest-api-specs`: maps are pointers

* `importer-rest-api-specs`: dereference more maps

* `importer-rest-api-specs`: remove use of strings.Title, dereference more maps

* limit scope of SDK generation test to the SDK being tested

* `importer-rest-api-specs`: two loops become one

* `importer-rest-api-specs`: workaround inconsistent casing of "VirtualWANs"

* Revert "`importer-rest-api-specs`: two loops become one"

This reverts commit 31b45ca.

* `importer-rest-api-specs`: normalize "attachednetworks" segment of resource IDs

* Revert "`tools/data-api-sdk`: making IsCommonType required so this is more explicit"

This reverts commit 71a166f.

* importer-rest-api-specs: use updated forks for `go-openapi/analysis`, `go-openapi/jsonreference`, and `go-openapi/spec`

* importer-rest-api-specs: remove Supplementary Data parser as is not used/needed

* importer-rest-api-specs: rework the swagger spec parser and manually resolve all remote references

* importer-rest-api-specs: fix format string, ensure model names are URL-decoded when parsing from the Ref URL fragment

* importer-rest-api-specs: updated tests for discriminated models to expect full ancestry

* importer-rest-api-specs: remove vestiges of supplementary data spike

* importer-rest-api-specs: do not set the description, see hashicorp#4434

* importer-rest-api-specs: hack for conflicting constant names in the same set of resources

* importer-rest-api-specs: disable check for non-default Resource Provider when parsing operations, as this was never utilised and leads to skipping resources/operations

* upper case URL in the go sdk

---------

Co-authored-by: Tom Bamford <tom@bamford.io>
Co-authored-by: Steph <steph@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tool/importer-rest-api-specs Swagger Data Importer issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants