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

Integration branch: OpenAPI Client generation #353

Merged
merged 16 commits into from
Oct 12, 2023
Merged

Conversation

emerkle826
Copy link
Contributor

This is the aggregate PR to merge all of the smaller OpenAPI client generation updates into the main branch once it's all finished.

@github-actions
Copy link

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

@Miles-Garnsey Miles-Garnsey changed the title OpenAPI Client generation Integration branch: OpenAPI Client generation Aug 22, 2023
@emerkle826 emerkle826 force-pushed the feature/client-gen branch 3 times, most recently from 0f46b5a to 6b965aa Compare September 15, 2023 15:29
@Miles-Garnsey
Copy link
Member

One note on this PR for posterity: we were unable to provide adequate testing around the v2 repairs endpoint because (as discussed here) we could not test the generated client due to limitations with Maven.

Reviewers should note that (while we could technically test this using an HTTP-only testing approach) it seems late in the game to do so, and my understanding is that this is urgently needed to meet our next milestone.

@adejanovski adejanovski marked this pull request as ready for review October 12, 2023 11:21
Copy link
Contributor Author

@emerkle826 emerkle826 left a comment

Choose a reason for hiding this comment

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

Just one nit suggestion. Not going to hold up the review for that.

@@ -51,6 +82,14 @@ public void setStatus(JobStatus status) {
this.status = status;
}

public void setStatusChange(ProgressEventType type, String message) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

suggestion:
This is probably better named addStatusChange

emerkle826 and others added 16 commits October 12, 2023 13:01
…hema (#346)

* Add currentStreams DTO
* add cluster name into endpoints response
* Add repair job version which follows notifications

* Add new getStatusChanges and message

* Fix formatting

* Add tests for the async endpoint in v1 also

* Change setStatusChange

* Fix test indentation

* Rename RpcParam

* Fix rebase

* Add back the payload in the IT test to the request

* Update openapi.json, make special case for return id 0 to the Rpc call - should fix the test also which can not be run otherwise on single node cluster

* Disable the test, throw exception if Cassandra refuses to repair

* Fix repair test to run repair for a keyspace with RF 2 (#351)

* Update openapi.json after rebase

---------

Co-authored-by: Erik Merkle <[email protected]>
* Allow `repair()` RPC method to take more arguments (as required by Reaper). Get the old NodeOpsResources class calling the new RPC method signature.

---------

Co-authored-by: Erik Merkle <[email protected]>

* Cancel repairs endpoint (#368)

* Add cancelAllRepairs endpoint and requisite NodeOps.
…h BigIntegers being represented in open API spec as ints. (#370)
* Correctly serialize and deserialize the statusChanges field

* Use escaped JSON instead when returning from getJobStatus
* Fix bug in getRingRangeString(), add unit test, add integration test.
* Make sure getJobStatus named consistently.
* Ensure status changes are serialised as JSON arrays, not as string.
@emerkle826 emerkle826 merged commit 2f15d16 into master Oct 12, 2023
8 of 164 checks passed
@emerkle826 emerkle826 deleted the feature/client-gen branch October 12, 2023 18: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.

5 participants