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

[2/6] [oxql] move core types to a new oxql-types crate #6364

Merged

Conversation

sunshowers
Copy link
Contributor

@sunshowers sunshowers commented Aug 16, 2024

Means that the Nexus external API crate doesn't have to pull in oximeter-db.

I did have to expose a few mutators, but my general reasoning is that because these types derive Deserialize, it's always possible to make these changes by serializing into JSON and working with them there.

Depends on #6355.

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1

[skip ci]
Created using spr 1.3.6-beta.1
sunshowers added a commit to sunshowers/omicron that referenced this pull request Aug 17, 2024
Means that the Nexus external API crate doesn't have to pull in oximeter-db.

Pull Request: oxidecomputer#6364
@sunshowers sunshowers changed the title [oxql] move core types to a new oxql-types crate [2/6] [oxql] move core types to a new oxql-types crate Aug 17, 2024
@sunshowers sunshowers requested a review from bnaecker August 17, 2024 02:39
Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

Wow, this one looks non-trivial, thanks for digging into it! I have one request, that we move TimeseriesKey around a bit, since it's more than an OxQL concept. Other than that, LGTM!

openapi/nexus.json Show resolved Hide resolved
oximeter/oxql-types/src/lib.rs Outdated Show resolved Hide resolved
@sunshowers sunshowers changed the base branch from sunshowers/spr/main.oxql-move-core-types-to-a-new-oxql-types-crate to main August 17, 2024 03:12
Created using spr 1.3.6-beta.1
Copy link
Collaborator

@bnaecker bnaecker left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for moving over the TimeseriesKey type!

@sunshowers sunshowers merged commit 4e8c731 into main Aug 18, 2024
25 checks passed
@sunshowers sunshowers deleted the sunshowers/spr/oxql-move-core-types-to-a-new-oxql-types-crate branch August 18, 2024 04:47
sunshowers added a commit that referenced this pull request Aug 28, 2024
At long last, here's the last OpenAPI document converted into a trait: the
Nexus external API. This is a fairly large change, but with all the
prerequisites in place the conversion is overall pretty mechanical. I did need
to move some types around, but the bulk of the PR is just adding the very large
external API trait.

This PR also adds a `.git-blame-ignore-revs` entry corresponding to #6372, so that the indent doesn't ruin blame in the GitHub web view. (You can configure this locally as well with `git config blame.ignoreRevsFile .git-blame-ignore-revs`.)

In a subsequent PR I'll remove the old instructions.

Depends on:

* #6355
* #6364 
* #6369
* #6370
* #6372

Co-authored-by: David Crespo <[email protected]>
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