bugfix: API: encode empty Vector/Matrix as [] not null #13997
Merged
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
bboreham
force-pushed
the
api-marshalling
branch
2 times, most recently
from
06eb3a5
to
b04fc88
Compare
|
If the code is not exported it can be removed |
It is registered with jsoniter as a side-effect of importing the package, so we don't know if someone expects it to be there. |
|
Could we change the default branch to |
|
@bboreham, I believe a few extra commits were added to the PR after you switched the base branch. Do you mind adjusting it? |
Since we already use require.JSONEq in similar cases. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
So that tests use the same encoding as the api. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
When we want to check just the json encoding. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
The typed versions are used when we call from one marshaller to another. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
If the underlying data is `nil` the default encoding will render `"null"` which is not accepted by (some) Prometheus client libraries. Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
bboreham
force-pushed
the
api-marshalling
branch
from
b04fc88
to
5c8ffaa
Compare
|
Sorry, have rebased and force-pushed. |
ArthurSens
approved these changes
Apr 29, 2024
|
Can we also merge this on main? :) |
|
Im trying to get the release out today, just need some approvals 😬 I'll open a PR to merge the release branch back to main immediately after |
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.
This is an alternative approach to fix the problem described in #13993 - where a query returns no data we should return
and not:
The key idea is to add marshallers for
VectorandMatrixinjson_codec.gowhich will achieve this effect, regardless of whether the underlying data isnilor a zero-length slice.However I discovered that the api tests don't actually use these marshallers, so some modifications were needed to address that.
After these changes, we probably don't need to register
unsafeMarshalFPointJSON,unsafeMarshalHPointJSONandunsafeMarshalSampleJSON, since Prometheus doesn't attempt to marshal single instances of those objects. But I left the code in place to avoid surprises, perhaps in a 3rd-party downstream.