Improve Ownership Ergonomics of PromqlResult and Related Types #9
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 3 commits
December 1, 2023 15:50
The EnumAsInner derive macro automatically generates methods like as_*()/as_*_mut() or into_*(), providing mutable and owned types whenever required.
This avoids cloning the data when ownership is required, as the previous API only allowed immutable references to the internal data.
Since the inner types implement Copy, this improves the ergonomics by making Sample also Copy.
|
Thank you for the PR. I will merge it and probably cut a new release by the end of the week. Right now I am a little busy with the day job. |
Great! Let me know if I need to make any further changes to the PR. |
|
I merged your PR and released a new version that contains your changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The existing library API only provides immutable references to the internal data of the Promql query result. This may be good for encapsulation but it becomes very limiting when ownership of the data is required. In the current implementation, owning the data requires cloning it, which can be very expensive if the data is large.
This PR attempts to mitigate that by allowing the user of the library to own the internal types by explicitly calling the relevant
into_*methods.Changes
as_variantandis_variantmethods on theDatatype have been removed. Instead, the EnumAsInner derive macro is used to generate the same methods automatically. Additionally this also adds theinto_variantmethods which allow user to own the internal types.PromqlResult,InstantVector, andRangeVectorimplementinto_innermethodSamplenow implementsCopy