Skip to content

Change the REST call for delete CloudConfig #882

Merged
junkaixue merged 2 commits intoapache:helix-cloudfrom
alirezazamani:helix-cloud-doc
Mar 13, 2020
Merged

Change the REST call for delete CloudConfig #882
junkaixue merged 2 commits intoapache:helix-cloudfrom
alirezazamani:helix-cloud-doc

Conversation

@alirezazamani
Copy link

Issues

Description

  • Here are some details about my PR, including screenshots of any UI changes:
    In this PR, the necessary changes has been made to the REST API call for cloud config deletion.
    Previously we were using POST (command.delete) to delete the cloud config. With this new change, we can use DELETE request to delete the cloud config.
    The tests has been changed accordingly.

Tests

  • The following is the result of the "mvn test" command on the appropriate module:

helix-core:
[INFO] Results:
[INFO]
[INFO] Tests run: 906, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 57:50 min
[INFO] Finished at: 2020-03-09T21:16:59-07:00
[INFO] ------------------------------------------------------------------------

helix-rest:
[INFO] Tests run: 99, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 26.976 s - in TestSuite
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 99, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 32.417 s
[INFO] Finished at: 2020-03-09T21:32:04-07:00
[INFO] ------------------------------------------------------------------------

Commits

  • My commits all reference appropriate Apache Helix GitHub issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Code Quality

  • My diff has been formatted using helix-style.xml

@junkaixue
Copy link
Contributor

Wait... Sorry for the confusing here. You have delete REST config in HelixAdmin. Then you dont need to move the whole logic to config accessor. In the ClusterAccesor, when you delete the REST config, what you need to do is call HelixAdmin's delete REST config.

@junkaixue
Copy link
Contributor

But we need update/delete operation for per entry in REST config.

Use DELETE instead of POST for deletion of the CloudConfig in REST.
Copy link
Contributor

@junkaixue junkaixue left a comment

Choose a reason for hiding this comment

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

Overall, LGTM. Please remove the line I commented.

@alirezazamani
Copy link
Author

This PR is ready to be merged, approved by @dasahcc.

Final commit message:

Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.

@junkaixue junkaixue merged commit 2104a89 into apache:helix-cloud Mar 13, 2020
zhangmeng916 pushed a commit to zhangmeng916/helix that referenced this pull request Apr 6, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
junkaixue pushed a commit that referenced this pull request Apr 7, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
junkaixue pushed a commit that referenced this pull request Apr 14, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
@alirezazamani alirezazamani deleted the helix-cloud-doc branch June 8, 2020 21:11
huizhilu pushed a commit to huizhilu/helix that referenced this pull request Aug 16, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
zhangmeng916 pushed a commit to zhangmeng916/helix that referenced this pull request Aug 25, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
zhangmeng916 pushed a commit to zhangmeng916/helix that referenced this pull request Nov 12, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
zhangmeng916 pushed a commit to zhangmeng916/helix that referenced this pull request Nov 18, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
jiajunwang pushed a commit that referenced this pull request Nov 18, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
jiajunwang pushed a commit that referenced this pull request Nov 19, 2020
Change the delete CloudConfig

Use DELETE instead of POST for deletion of the CloudConfig in REST.
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