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

Adds UI for shrink action #176

Merged
merged 6 commits into from
Apr 18, 2022
Merged

Conversation

downsrob
Copy link
Contributor

@downsrob downsrob commented Apr 15, 2022

Signed-off-by: Clay Downs downsrob@amazon.com

Description

Adds UI for the shrink action. This PR does not contain any tests, which should be added later. Additional validation should also be added, such as not allowing percentage to shrink shards to input out of the range of 0 to 1. This is currently validated by the backend but additional validation up front would improve the user experience.

Zoomed out overview of the visual editor:

Screen Shot 2022-04-14 at 7 41 15 PM

If more than one number of shards setting is added, this error appears:

Screen Shot 2022-04-14 at 7 41 49 PM

When either of the JSON editors are poorly formatted it will show as red.

Screen Shot 2022-04-14 at 7 42 25 PM
If any of the above errors are present, it will not be possible to add the action.

Template and alias updated to say -optional

Screen Shot 2022-04-15 at 3 46 12 PM

Warning that appears for force unsafe option

Screen Shot 2022-04-15 at 4 29 26 PM

Issues Resolved

opensearch-project/index-management#40

Check List

  • 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.

Signed-off-by: Clay Downs <downsrob@amazon.com>
@downsrob downsrob requested a review from a team April 15, 2022 03:04
Signed-off-by: Clay Downs <downsrob@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 15, 2022

Codecov Report

Merging #176 (b54eb82) into main (7352cdb) will decrease coverage by 1.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #176      +/-   ##
==========================================
- Coverage   63.32%   62.28%   -1.05%     
==========================================
  Files         155      156       +1     
  Lines        4447     4552     +105     
  Branches      738      718      -20     
==========================================
+ Hits         2816     2835      +19     
- Misses       1415     1500      +85     
- Partials      216      217       +1     
Impacted Files Coverage Δ
...mponents/EuiFormCustomLabel/EuiFormCustomLabel.tsx 80.00% <0.00%> (-7.50%) ⬇️
...n/public/pages/VisualCreatePolicy/utils/helpers.ts 68.36% <0.00%> (-0.71%) ⬇️
...public/pages/VisualCreatePolicy/utils/constants.ts 100.00% <0.00%> (ø)
...eatePolicy/components/UIActions/ShrinkUIAction.tsx 16.83% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7352cdb...b54eb82. Read the comment docs.

onChange={(id) => {
const forceUnsafe = id === "yes";
const shrinkObject = { ...action.action };
if (forceUnsafe) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not critical but may be doing a popup or having some warning when this is set to true stating can potentially result in data loss would be preferred I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added and demoed in a new screenshot

}
};

render = (action: UIAction<ShrinkAction>, onChangeAction: (action: UIAction<ShrinkAction>) => void) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, how we are doing for other actions but may be differentiating optional and required fields by an * or calling it out in words might be helpful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added and demoed in a new screenshot

thalurur
thalurur previously approved these changes Apr 15, 2022
Signed-off-by: Clay Downs <downsrob@amazon.com>
Signed-off-by: Clay Downs <downsrob@amazon.com>
Signed-off-by: Clay Downs <downsrob@amazon.com>
Signed-off-by: Clay Downs <downsrob@amazon.com>
@downsrob downsrob requested a review from thalurur April 16, 2022 18:16
@downsrob downsrob merged commit 85f8e13 into opensearch-project:main Apr 18, 2022
@downsrob downsrob deleted the shrink-action branch April 18, 2022 15:44
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.

4 participants