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

chore(epic-idpe-17711): use SQL HTTP API to get field list #6790

Merged

Conversation

appletreeisyellow
Copy link
Contributor

@appletreeisyellow appletreeisyellow commented Aug 22, 2023

Wait for #6783 to merge

Part of https://github.com/influxdata/idpe/issues/17981

This PR uses the SQL HTTP API to query fields

The change is behind the feature flag v2privateQueryUI and it is set to false by default

To test locally

  1. Deploy the remocal with the following flag:
    make deploy-remocal DEPLOY_IOX=true ENABLE_IOX_FLIGHT=true
    
  2. Then you will see the query for measurement list will send to /api/v2private/query in DevTool Network tab

Before and After

Screen.Recording.2023-08-22.at.4.16.40.PM.mov

Checklist

Authors and Reviewer(s), please verify the following:

  • A PR description, regardless of the triviality of this change, that communicates the value of this PR
  • Well-formatted conventional commit messages that provide context into the change
  • Documentation updated or issue created (provide link to issue/PR)
  • Signed CLA (if not already signed)
  • Feature flagged, if applicable

@appletreeisyellow appletreeisyellow self-assigned this Aug 23, 2023
@appletreeisyellow appletreeisyellow marked this pull request as ready for review August 25, 2023 18:48
@appletreeisyellow appletreeisyellow requested a review from a team as a code owner August 25, 2023 18:48
@appletreeisyellow appletreeisyellow changed the title chore(schemaBrowser): use SQL HTTP API to get field list chore(epic-idpe-17711): use SQL HTTP API to get field list Aug 25, 2023
Copy link
Contributor

@wdoconnell wdoconnell 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. One minor non-blocking comment re: one type cast.

bucket,
} as QueryOptions)
const values = (resp.parsed.table?.columns?.column_name?.data ??
[]) as string[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the reason we don't get strong typing here (and have to type cast using as) that we can't predict the type of resp.parsed.table.columns.column_name.data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wdoconnell We actually can predict the type of resp.parsed.table.columns.column_name.data which is string[] | number[] | boolean[]. Since field keys are strings, so we cast the data to string[] specifically

@appletreeisyellow appletreeisyellow added this pull request to the merge queue Aug 28, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 28, 2023
@appletreeisyellow appletreeisyellow added this pull request to the merge queue Aug 28, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 28, 2023
@appletreeisyellow appletreeisyellow added this pull request to the merge queue Aug 30, 2023
Merged via the queue into master with commit fb1876b Aug 30, 2023
2 checks passed
@appletreeisyellow appletreeisyellow deleted the chunchun/v2privatequery-schema-browse-fields branch August 30, 2023 15:33
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