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

tools/importer-rest-api-specs: refactoring the Parser package #4307

Merged
merged 58 commits into from
Oct 9, 2024

Conversation

tombuildsstuff
Copy link
Contributor

@tombuildsstuff tombuildsstuff commented Jul 15, 2024

This PR refactors the Parser package into apidefinitions - and removes the old logic/pipeline

Right now this mostly works and requires a few fixes around Discriminators/Supplementary Data that @manicminer is going to pick up

Mostly fixes/rounds out #3754

@tombuildsstuff tombuildsstuff changed the title WIP tools/importer-rest-api-specs: refactoring the Parser package Jul 15, 2024
@manicminer manicminer marked this pull request as ready for review July 19, 2024 00:05
@manicminer manicminer added tool/importer-rest-api-specs Swagger Data Importer issues refactor labels Jul 19, 2024
stephybun
stephybun previously approved these changes Jul 31, 2024

Where possible we use the terminology `API Definition` since these are implementation details - but this more specific terminology will be used where necessary.

TODO also mention Supplemental Data
Copy link
Member

Choose a reason for hiding this comment

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

@manicminer do you have context on this?

// if this is a parent model, look for implementations. this will currently only work for children defined in
// the same swagger file.
Copy link
Member

Choose a reason for hiding this comment

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

Do we handle the case where the children are defined in a different swagger file somewhere else in build.go?

Context: #3727

Copy link
Contributor

Choose a reason for hiding this comment

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

@manicminer manicminer force-pushed the refactor/importer-rest-api-specs-iv branch from f57a3a2 to 23eccdf Compare October 1, 2024 11:13
@manicminer manicminer assigned stephybun and unassigned manicminer Oct 1, 2024
…e 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
…to the new package

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

There's a bunch of these, and whilst these'll get replaced in time with #4017
but for now this cleans this up
…since it doesn't seem to be required for DataFactory models as they use external Swagger Refs.
… `go-openapi/jsonreference`, and `go-openapi/spec`
…L-decoded when parsing from the Ref URL fragment
…der when parsing operations, as this was never utilised and leads to skipping resources/operations
@stephybun stephybun force-pushed the refactor/importer-rest-api-specs-iv branch from d9f992a to 1da6920 Compare October 9, 2024 10:07
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.

LGTM 🪀

@stephybun stephybun merged commit 2439909 into main Oct 9, 2024
3 checks passed
@stephybun stephybun deleted the refactor/importer-rest-api-specs-iv branch October 9, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor tool/importer-rest-api-specs Swagger Data Importer issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants