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

Add support for RepairRun REST resource #6

Merged
merged 15 commits into from
Jun 17, 2021
Merged

Add support for RepairRun REST resource #6

merged 15 commits into from
Jun 17, 2021

Conversation

adutra
Copy link
Contributor

@adutra adutra commented Jun 2, 2021

While API design is being discussed in #5, I'm opening this PR to get the ball rolling. If the design principles change, I will amend it accordingly.

This PR contains a few commits so far:

  • A few preparatory commits:
    • Run go mod tidy c8aae7e
    • Rename main interface and implementing struct ad9e736
    • Refactor HTTP internals 1a96163
    • Move API members to files grouped by REST resources 69afba2
  • Support for RepairRun REST resource cd1f458

Commits can be reviewed separately.

┆Issue is synchronized with this Jira Bug by Unito

reaper/repair_run_test.go Outdated Show resolved Hide resolved
// CreateRepairRun creates a new repair run for the given cluster and keyspace. Does not actually trigger the run:
// creating a repair run includes generating the repair segments. Returns the id of the newly-created repair run if
// successful. The owner name can be any string identifying the owner.
CreateRepairRun(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this is a question for @adejanovski. When I create a RepairRun, do I typically want to start it?

Copy link
Contributor

Choose a reason for hiding this comment

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

It depends 😅
So, I'd say not necessarily instantly if you first want to check how many segments you end up with.

reaper/cluster.go Outdated Show resolved Hide resolved
reaper/cluster.go Outdated Show resolved Hide resolved
reaper/cluster.go Outdated Show resolved Hide resolved
reaper/cluster.go Outdated Show resolved Hide resolved
reaper/cluster.go Outdated Show resolved Hide resolved
@adutra
Copy link
Contributor Author

adutra commented Jun 17, 2021

@adejanovski @jeffbanks or @jsanda if everything looks good, would you mind giving your approval please?

Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

You still have some methods that start with "Get", but I think it's not such a big deal and we can move on with this PR.
We can deal with this in a future refactoring.

@adutra
Copy link
Contributor Author

adutra commented Jun 17, 2021

You still have some methods that start with "Get", but I think it's not such a big deal and we can move on with this PR.
We can deal with this in a future refactoring.

Oh you are absolutely right, my bad. I can offer to change the RepairRun-related methods real quick. The I'd suggest to leave the pre-existing methods (GetCluster, etc...) as is and change them later in a follow-up PR.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 16 Code Smells

No Coverage information No Coverage information
1.9% 1.9% Duplication

@adejanovski
Copy link
Contributor

Thanks for making the change @adutra ! LGTM ✅

@adutra adutra merged commit fe2ba92 into master Jun 17, 2021
@adutra adutra deleted the repair-run branch June 17, 2021 11:19
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