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

Jump to bottom

mgr/dashboard: fix snapshot creation with duplicate name #47976

Merged
merged 1 commit into from Sep 12, 2022
Merged

mgr/dashboard: fix snapshot creation with duplicate name #47976

merged 1 commit into from Sep 12, 2022

Conversation

aaSharma14
Copy link
Contributor

@aaSharma14 aaSharma14 commented Sep 5, 2022 •
edited

Snapshot creation with same name on UI throwing 500 Internal Error, This PR intends to fix this issue.

Fixes: https://tracker.ceph.com/issues/57456
Signed-off-by: Aashish Sharma aasharma@redhat.com

Screenshot from 2022-09-05 17-28-04

Screenshot from 2022-09-06 10-57-37

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@aaSharma14 aaSharma14 requested a review from a team as a code owner September 5, 2022 12:03
@aaSharma14 aaSharma14 added this to In progress in Dashboard via automation Sep 5, 2022
@github-actions github-actions bot added the pybind label Sep 5, 2022
Dashboard automation moved this from In progress to Reviewer approved Sep 5, 2022
epuertat
epuertat approved these changes Sep 5, 2022
View reviewed changes
Copy link
Member

@epuertat epuertat left a comment

Choose a reason for hiding this comment

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

This fixes the UI validation, but what happens if I try this same operation from the API?

@aaSharma14 aaSharma14 force-pushed the fix-duplicate-snapshot branch 3 times, most recently from 5148e9f to ad8b8cc Compare September 6, 2022 05:38
@aaSharma14
Copy link
Contributor Author

This fixes the UI validation, but what happens if I try this same operation from the API?

@epuertat , Added API validation as well

@nizamial09
Copy link
Member

This fixes the UI validation, but what happens if I try this same operation from the API?

@epuertat , Added API validation as well

@aaSharma14 just a doubt, if we added the API validation, then do we need a frontend validation? I mean the backend will already throw the Exception right and it'll be caught and shown in the frontend as well. Maybe an async validator in the form name field will be good IMHO.

@pereman2
Copy link
Contributor

pereman2 commented Sep 6, 2022

jenkins test make check

@aaSharma14
Copy link
Contributor Author

This fixes the UI validation, but what happens if I try this same operation from the API?

@epuertat , Added API validation as well

@aaSharma14 just a doubt, if we added the API validation, then do we need a frontend validation? I mean the backend will already throw the Exception right and it'll be caught and shown in the frontend as well. Maybe an async validator in the form name field will be good IMHO.

@nizamial09 , I think by adding the validation to the frontend as well will prevent sending the API request at first place

1 similar comment
@aaSharma14
Copy link
Contributor Author

This fixes the UI validation, but what happens if I try this same operation from the API?

@epuertat , Added API validation as well

@aaSharma14 just a doubt, if we added the API validation, then do we need a frontend validation? I mean the backend will already throw the Exception right and it'll be caught and shown in the frontend as well. Maybe an async validator in the form name field will be good IMHO.

@nizamial09 , I think by adding the validation to the frontend as well will prevent sending the API request at first place

@aaSharma14
mgr/dashboard: fix snapshot creation with duplicate name
2ae1bca 
Snapshot creation with same name on UI throwing 500 Internal Error, This PR intends to fix this issue.

Fixes: https://tracker.ceph.com/issues/57456
Signed-off-by: Aashish Sharma <aasharma@redhat.com>
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard cephadm

@aaSharma14
Copy link
Contributor Author

jenkins test windows

@aaSharma14
Copy link
Contributor Author

jenkins test dashboard cephadm

1 similar comment
@aaSharma14
Copy link
Contributor Author

jenkins test dashboard cephadm

@nizamial09 nizamial09 merged commit 6ef176b into ceph:main Sep 12, 2022
14 checks passed
Dashboard automation moved this from Reviewer approved to Done Sep 12, 2022
@nizamial09 nizamial09 deleted the fix-duplicate-snapshot branch September 12, 2022 05:52
@aaSharma14 aaSharma14 mentioned this pull request Sep 12, 2022
Merged
@aaSharma14 aaSharma14 mentioned this pull request Sep 12, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@epuertat epuertat epuertat approved these changes

@nizamial09 nizamial09 nizamial09 approved these changes

@pereman2 pereman2 Awaiting requested review from pereman2

Assignees
No one assigned
Labels
dashboard pybind
Projects
Ceph-Dashboard
Archived in project
Dashboard
  
Done
Milestone
No milestone
4 participants
@aaSharma14 @nizamial09 @pereman2 @epuertat