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

Variables: add variable support #77

Closed
wants to merge 10 commits into from
Closed

Variables: add variable support #77

wants to merge 10 commits into from

Conversation

ryantxu
Copy link
Member

@ryantxu ryantxu commented Jan 25, 2021

support for query variables. Related to #113

image

@ryantxu ryantxu requested a review from bymattoa January 25, 2021 23:12
@ryantxu
Copy link
Member Author

ryantxu commented Jan 25, 2021

@bymattoa -- most of this PR is formatting changes due to an upgrade in prettier :(

return this.datasource.query(request).pipe(
mergeMap((rsp) => {
const assets = new DataFrameView<HierarchyInfo>(rsp.data[0]);
const data: MetricFindValue[] = assets.map((a) => ({
Copy link
Member Author

Choose a reason for hiding this comment

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

The default behavior just uses the string values as the variables... BUT for this, I assume we actually want the ids as the template value and the name as what is displayed

Copy link
Contributor

Choose a reason for hiding this comment

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

We will also need to fetch property IDs. (See thread in conversation)

Copy link
Contributor

@bymattoa bymattoa left a comment

Choose a reason for hiding this comment

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

We're also going to need to fetch property IDs (which are available on models) as well as asset IDs. How would a user go about mapping these?

@ryantxu
Copy link
Member Author

ryantxu commented Feb 2, 2021

This now uses the "Describe Asset" query to pick properties:

image

@ryantxu
Copy link
Member Author

ryantxu commented Feb 2, 2021

NOTE: the "Explore" button does not work -- need to investigate why, but It does not look like an easy easy fix 😢

should we merge without that fix first?

@ryantxu ryantxu requested a review from bymattoa February 2, 2021 22:41
@toddtreece
Copy link
Member

i think #134 and #127 cover the changes needed for #113. closing this one

@toddtreece toddtreece closed this Jun 6, 2022
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.

None yet

3 participants