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

closes #320: implement findCollections command #340

Merged
merged 4 commits into from
Apr 17, 2023

Conversation

ivansenic
Copy link
Contributor

What this PR does:
Implements the findCollections command.. Lists only jsonapi collections in a namespace. Example similar to findNamespaces:

$ curl -X 'POST' \
>   'http://localhost:8080/v1/cycling' \
>   -H 'accept: application/json' \
>   -H 'X-Cassandra-Token: 90e68c40-c254-43cb-87d2-91c9003de546' \
>   -H 'Content-Type: application/json' \
>   -d '{
>   "findCollections": {}
> }' | jq .

{
  "status": {
    "existingCollections": [
      "events"
    ]
  }
}

Which issue(s) this PR fixes:
Fixes #320

@ivansenic ivansenic requested a review from a team as a code owner April 6, 2023 12:14
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

LGTM overall, one trivial suggestion

@ivansenic ivansenic force-pushed the ise-320-find-collections branch from c794db5 to e94d0b7 Compare April 7, 2023 08:43
@ivansenic
Copy link
Contributor Author

@johnsmartco we would also need spec for this new command as well..

import javax.inject.Inject;

@ApplicationScoped
public class CollectionManager {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think adding this class was not needed.. I can simply use the SchemaManager in the ops.. I see no future need for extending this one, thus I might remove it..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@ivansenic ivansenic force-pushed the ise-320-find-collections branch from e94d0b7 to ddce150 Compare April 7, 2023 09:02
@jeffreyscarpenter jeffreyscarpenter marked this pull request as draft April 12, 2023 23:09
@jeffreyscarpenter
Copy link
Contributor

Converted to draft as suggested by @maheshrajamani

@ivansenic
Copy link
Contributor Author

@jeffreyscarpenter @maheshrajamani This was ready to review a long time ago, I don't understand why is this moved to draft and still not reviewed?

@ivansenic ivansenic marked this pull request as ready for review April 17, 2023 08:25
@ivansenic ivansenic force-pushed the ise-320-find-collections branch from ddce150 to 5844db6 Compare April 17, 2023 10:18
@maheshrajamani
Copy link
Contributor

@jeffreyscarpenter @maheshrajamani This was ready to review a long time ago, I don't understand why is this moved to draft and still not reviewed?

@ivansenic I have suggested to move it to draft because of your comment to "add test for this class" and also I did not find any unit test cases for those classes.

Copy link
Contributor

@maheshrajamani maheshrajamani left a comment

Choose a reason for hiding this comment

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

LGTM

@ivansenic ivansenic merged commit c841b16 into main Apr 17, 2023
@ivansenic ivansenic deleted the ise-320-find-collections branch April 17, 2023 13:11
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.

Implement findCollections command
4 participants