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

Feature/sc 175067 add summary type selector #40

Conversation

alexbourret
Copy link
Contributor

Copy link

Copy link

@liamlynch-data liamlynch-data left a comment

Choose a reason for hiding this comment

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

Two bugs noticed, the one already mentioned with the Search attributes connector, also one with event frames search (basically impl missing there).

Something else I noticed - when going between Master and this branch, in the search attribute connector in master when Summary data type is selected, the item was not unpacked so the value column had things like this in it:
{'Timestamp': '2024-08-27T08:38:47.0143941Z', 'Value': 5116.335696505253, 'UnitsAbbreviation': '', 'Good': True, 'Questionable': False, 'Substituted': False, 'Annotated': False}

This is fixed here which is good (maybe not by this PR, might be a previous change, not sure). The question is, is this change in behaviour risky? I don't think so, it is more of a fix as Summary was not really usable before - just thought I would mention it.

@liamlynch-data
Copy link

liamlynch-data commented Aug 30, 2024

Question - for the assets downloader recipe, shouldn't the starttime and endtime fields appear in the UI when summary data type is selected?
They do for the connector when retrieving values.

image

image

@alexbourret
Copy link
Contributor Author

Made the start / end time selectors visible for summary in the recipe with e9ba9d3

…nual-input-on-attribute-search-connector

Feature/sc 190307 add template manual input on attribute search connector
…value-drift

Bug/sc 188829 pi time expression value drift
@alexbourret alexbourret merged commit 7f9d9c1 into feature/sc-159069-no-more-maxcount Sep 2, 2024
1 check passed
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