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

fix validation for certain cases #771

Closed

Conversation

wang-chenxi
Copy link
Contributor

@wang-chenxi wang-chenxi commented Oct 18, 2023

Description

Worked on issue 671 add validation to trigger name

After initial research, I realized there is an existing validation to be used for multiple places, however, it doesnt work when creating new monitors. I have checked the parms and realized within the validation func, it compares empty items so it would never return a validation for existing name.

Also, this feature is more complex than what it described, as there's four different monitors to be created and which would store the data in different structure, the existing validation funcs use a single logic to grab the items.

Issues Resolved

This PR refactored existing validation, with a flag from upper level to understand if we are creating a new monitor that wont be with ids for triggers. Also, it refactored the utils of values to make sure it wont be an empty array forever.

This PR covers cases of query monitor and cluster monitor, which is able to grab the underconstrution monitors and then use index to check each monitor's name in a loop.

I would like to get more clarifications on how the other tow monitors's data structured. If cannot get more information, i would purpose this PR to resolve the validation partly, and my next PR will resolve the other two, as it would take some time to understand the data structure and logic.

This PR doesnt include test as of now, since validation is not a new functionality, it just didnt work correctly and i fixed the problem. But feel free to let me know if you need tests.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@wang-chenxi wang-chenxi closed this Nov 6, 2023
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.

1 participant