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

Relax NRQL condition limitations #136

Closed

Conversation

RobDay-Reynolds
Copy link

NRQL condition limitations for duration and since_value terms are previously overly strict and limiting out values that were perfectly acceptable for the API.

duration is previously limiting to one of 1, 2, 3, 4, 5, 10, 15, 30, 60, or 120, but the API has no such restriction and only requires that it is a positive integer

since_value is limited to "1", "2", "3", "4", or "5", but the API allows any value between 1 and 20.

This PR relaxes limitations on the term validation to more accurately reflect the limitations imposed by the API.

* API allows durations between 1 and 20
* Previously limited durations to be between 1 and 5
* API allows any positive integer
* Previously limited to 1, 2, 3, 4, 5, 10, 15, 30, 60, or 120
@ghost ghost added size/M documentation Improvements or additions to documentation labels May 10, 2019
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Found some fixes!

P.S. share your ideas, feedbacks or issues with us at https://github.com/fixmie/feedback (this message will be removed after the beta stage).

* Validation method should not be exported out of package
@ianrayns
Copy link
Contributor

Awesome, this addresses #131

@paultyng paultyng self-assigned this Jul 10, 2019
@paultyng
Copy link
Contributor

since_value was addressed in #144, didn't see this PR prior to merging, so going to modify to just do the other attribute.

@paultyng paultyng mentioned this pull request Jul 11, 2019
@ctrombley
Copy link
Contributor

This has been addressed in other PRs (#144, #178).

@ctrombley ctrombley closed this Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants