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

gh-175 Adds schema class exists method #196

Merged
merged 4 commits into from
Apr 28, 2023

Conversation

HeliosX7
Copy link
Contributor

Addresses #175
client.schema().exists().withClassName("clazz").run(); //returns Result<Boolean>

@weaviate-git-bot
Copy link

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the SeMI bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

private String className;

public ClassExists(HttpClient httpClient, Config config) {
this.classGetter = new ClassGetter(httpClient, config);
Copy link
Member

Choose a reason for hiding this comment

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

Avoid creating instances of dependencies inside classes. Instead create dependencies outside and inject them as arguments of constructor.

It will make testing a lot easier by using mocks instead of real implementations.

Copy link
Member

Choose a reason for hiding this comment

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

this. prefix is not necessary here

WeaviateErrorMessage errorMessage = WeaviateErrorMessage.builder()
.message("classname cannot be empty").build();
WeaviateErrorResponse errors = WeaviateErrorResponse.builder()
.error(Stream.of(errorMessage).collect(Collectors.toList())).build();
Copy link
Member

Choose a reason for hiding this comment

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

Stream.of(errorMessage).collect(Collectors.toList())
although correct, stream seems to be a bit excessive here.
Arrays.asList(errorMessage) or Collections.singletonList(errorMessage) can be used instead.

@HeliosX7
Copy link
Contributor Author

Addressed the review comments in latest commit

@HeliosX7 HeliosX7 requested a review from aliszka April 19, 2023 17:47
return this;
}

private Result<Boolean> toResult(WeaviateError error) {
Copy link
Member

Choose a reason for hiding this comment

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

not used at the moment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

missed it, removed now

Copy link
Member

@aliszka aliszka left a comment

Choose a reason for hiding this comment

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

Looks good!

@aliszka aliszka merged commit 1d7432b into weaviate:main Apr 28, 2023
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.

3 participants