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 tenant as an optional param to batch.delete_objects #375

Merged
merged 3 commits into from
Jul 10, 2023

Conversation

ciprianaradulescu
Copy link

@ciprianaradulescu ciprianaradulescu commented Jul 9, 2023

Fix for #374

@dirkkul
Copy link
Collaborator

dirkkul commented Jul 9, 2023

Hi @ciprianaradulescu ,

thank you for your contribution! Could you add a test under integration/test_batch.py? You could just extend test_add_object_batch_with_tenant

@ciprianaradulescu
Copy link
Author

Hello @dirkkul ,

I've extended test_add_object_batch_with_tenant to check for both a valid and an invalid batch delete based on the tenant. Please let me know if it looks ok.

Thank you !

@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 Weaviate bot 👋🤖

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

@ciprianaradulescu
Copy link
Author

I agree with the CLA.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 95.49% and project coverage change: -0.21 ⚠️

Comparison is base (2d3dd56) 95.63% compared to head (eb3bf16) 95.43%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
- Coverage   95.63%   95.43%   -0.21%     
==========================================
  Files          64       64              
  Lines        7836     8165     +329     
==========================================
+ Hits         7494     7792     +298     
- Misses        342      373      +31     
Impacted Files Coverage Δ
integration/test_grcp.py 98.59% <ø> (ø)
test/batch/test_crud_batch.py 99.22% <ø> (ø)
test/cluster/test_cluster.py 100.00% <ø> (ø)
test/data/test_crud_data.py 99.23% <ø> (ø)
weaviate/gql/filter.py 94.34% <ø> (ø)
weaviate/schema/validate_schema.py 100.00% <ø> (ø)
weaviate/warnings.py 89.36% <66.66%> (-8.37%) ⬇️
weaviate/connect/connection.py 88.93% <72.72%> (-1.24%) ⬇️
weaviate/schema/crud_schema.py 78.15% <72.72%> (-1.28%) ⬇️
weaviate/batch/crud_batch.py 95.32% <85.71%> (+2.78%) ⬆️
... and 18 more

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dirkkul
Copy link
Collaborator

dirkkul commented Jul 10, 2023

Hi @ciprianaradulescu, thank you, test looks good! Could you let black run over your code? The contributing guide shows how to do it.

I'll merge afterwards

@ciprianaradulescu
Copy link
Author

Yeah, sorry, missed that 😁 Should be fine now.

@dirkkul dirkkul merged commit b733212 into weaviate:main Jul 10, 2023
18 checks passed
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.

None yet

4 participants