-
Notifications
You must be signed in to change notification settings - Fork 16
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
Conversation
There was a problem hiding this 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
c794db5
to
e94d0b7
Compare
@johnsmartco we would also need spec for this new command as well.. |
import javax.inject.Inject; | ||
|
||
@ApplicationScoped | ||
public class CollectionManager { |
There was a problem hiding this comment.
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..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
...ain/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindCollectionsOperation.java
Outdated
Show resolved
Hide resolved
e94d0b7
to
ddce150
Compare
Converted to draft as suggested by @maheshrajamani |
@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? |
ddce150
to
5844db6
Compare
@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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What this PR does:
Implements the
findCollections
command.. Lists only jsonapi collections in a namespace. Example similar tofindNamespaces
:Which issue(s) this PR fixes:
Fixes #320