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

experimental/sdata: Add RefID to writers and readers #627

Merged
merged 2 commits into from
Feb 23, 2023
Merged

Conversation

kylebrandt
Copy link
Contributor

This makes it so Frames are creating with a RefID which was skipped before.

We do insert this in other parts of the SDK for datasources, but probably better to keep it clear, and I need to specify it in SSE.

@kylebrandt kylebrandt added the area/dataplane Dataplane Project (Data type contract) label Feb 23, 2023
@kylebrandt kylebrandt requested a review from a team February 23, 2023 12:29
@kylebrandt kylebrandt requested a review from a team as a code owner February 23, 2023 12:29
@kylebrandt kylebrandt requested review from marefr and andresmgot and removed request for a team, marefr and andresmgot February 23, 2023 12:29
Comment on lines +5 to 9
func emptyFrameWithTypeMD(refID string, t data.FrameType, v data.FrameTypeVersion) *data.Frame {
f := data.NewFrame("").SetMeta(&data.FrameMeta{Type: t, TypeVersion: v})
f.RefID = refID
return f
}
Copy link
Contributor

Choose a reason for hiding this comment

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

will there be any side effects using empty frame name? Should we fallback to refID as frame name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. The frame name is never given any meaning in any of these data types currently. So having it zero valued seems appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

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

By any chance transformations will break with inconsistent frame names? ( for a same query, valid results have different frame name when no results have empty frame name )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not aware of anything in transforms that says valid results must have a frame name?

image

image

Random sample of one transform seems to work fine?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea fine. Just we need to aware of this and document that the frame names can potentially empty or duplicate. so that consumers can use it foolproof.

@kylebrandt kylebrandt merged commit 933e4ca into main Feb 23, 2023
@kylebrandt kylebrandt deleted the sdataRefID branch February 23, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dataplane Dataplane Project (Data type contract)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants