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

v0.13.0 Introducing the Rolodex. #187

Merged
merged 75 commits into from
Nov 4, 2023
Merged

v0.13.0 Introducing the Rolodex. #187

merged 75 commits into from
Nov 4, 2023

Conversation

daveshanley
Copy link
Member

@daveshanley daveshanley commented Oct 24, 2023

The rolodex is the solution libopenapi has needed for some time. It reorganizes how the index works, how references are composed and how things are looked up when building the model.

The rolodex creates virtual filesystems for exploded specifications that are scattered across multiple local or remote locations. The rolodex can contain multiple local or remote filesystems for documents, and can lookup anything
via its fully qualified location.

Custom filesystems can be loaded in, instead of the local and remote ones provided by libopenapi as the rolodex uses fs.FS as an interface, so any custom implementation can be used.

This is a large breaking change for how the index operates, there are breaking changes to all low-level models, mainly with the introduction of a context.Context parameter. This is used by the rolodex to know where to resolve references from, when deeply nested recursive reference resolution.

The index no-longer operates the way it used to and how indexes are used by the rolodex is also new.

There should be minimal configuration changes at the top-level API for documents, and no changes to the high-level model.

Signed-off-by: quobix <[email protected]>
assembling the low level blocks, all based on `fs.FS` interfaces.

Signed-off-by: quobix <[email protected]>
To make this work correctly, this needs completely shaking up and a transfer of ownership. The index is now local,
the rolodex is now global.

Signed-off-by: quobix <[email protected]>
this is some complex and messy work.

Signed-off-by: quobix <[email protected]>
There is a horrible amount of work to be done to clean this up, and wire in remote support. but so far, this is working as expected and is now a much cleaner design, (once everything has been cleaned up that is)

Signed-off-by: quobix <[email protected]>
seems to be holding, more tests to change.

Signed-off-by: quobix <[email protected]>
Bringing the tests back online, bit by bit.

Signed-off-by: quobix <[email protected]>
time to start some cleanup.

Signed-off-by: quobix <[email protected]>
Sucking in all the files!

Signed-off-by: quobix <[email protected]>
starting the circle dance now.

Signed-off-by: quobix <[email protected]>
so many things that can go wrong. have to catch them all.

Signed-off-by: quobix <[email protected]>
It’s so, so much faster than before, intelligent and ready for scale. I’m excited!

Signed-off-by: quobix <[email protected]>
Before everything worked, but was completely accurate, now everything works and everything is absolute and can be resolved. Phew, what a mission!

Signed-off-by: quobix <[email protected]>
first round of a number I am sure, lots to clean.

Signed-off-by: quobix <[email protected]>
Every `Build()` method now requires a `context.Context`. This is so the rolodex knows where to resolve from when locating relative links. Without knowing where we are, there is no way to resolve anything. This new mechanism allows the model to recurse across as many files as required to locate references, without loosing track of where we are in the process.

Signed-off-by: quobix <[email protected]>
…le` signature.

All tests in index and datamodel now pass. The rolodex fixes all the things.

Signed-off-by: quobix <[email protected]>
Document configuration has been simplified, no more need for AllowRemote stuff in the document configuration, it’s assumed by setting the baseURL or the basePath.

Signed-off-by: quobix <[email protected]>
Also removed old swagger `CreateDocument` method that has been deprecated.

Signed-off-by: quobix <[email protected]>
This will be a bit of a slog, new code built in the hot path will need some love and attention.

Signed-off-by: quobix <[email protected]>
The remote loader was blocking the only thread.

Signed-off-by: quobix <[email protected]>
@daveshanley
Copy link
Member Author

Looks like the lovely single core runners are choking on locks.

cannot re-create the problem locally, even when setting GOMAXPROCS to 1

Signed-off-by: quobix <[email protected]>
We’re ready for review

Signed-off-by: quobix <[email protected]>
@daveshanley daveshanley marked this pull request as ready for review November 1, 2023 23:54
@daveshanley
Copy link
Member Author

For end users of the high-level model. You should notice no real changes. However this PR is loaded with breaking changes, I had to open up the heart of the library and add new bones to the skeleton.

Copy link
Contributor

@TristanSpeakEasy TristanSpeakEasy left a comment

Choose a reason for hiding this comment

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

Looking good to me, nothing too scary in here from a migration perspective. Going to start a branch in our product and give the upgrade a try and see if we find any early issues

props did not have context, therefore they had no idea where they were or where to resolve from.

Signed-off-by: quobix <[email protected]>
model was failing on subschemas with refs, needed context

Signed-off-by: quobix <[email protected]>
Lots and lots of variations. means lots of branches to check.

Signed-off-by: quobix <[email protected]>
Signed-off-by: quobix <[email protected]>

# Conflicts:
#	what-changed/model/info_test.go
@daveshanley
Copy link
Member Author

It's time to merge.

@daveshanley daveshanley merged commit 771baaf into main Nov 4, 2023
3 checks passed
@daveshanley daveshanley deleted the v0.13.0 branch November 4, 2023 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants