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 tests for Vector Collection backup config #711

Merged
merged 4 commits into from
Nov 15, 2024

Conversation

olukas
Copy link
Contributor

@olukas olukas commented Nov 14, 2024

Add tests for setting Vector Collection backup_count and async_backup_count configuration.

Copy link
Contributor

@yuce yuce left a comment

Choose a reason for hiding this comment

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

Just a few ideas...

name = random_string()
self.client.create_vector_collection_config(
name, [IndexConfig("vector", Metric.COSINE, 3)], backup_count=6
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to check whether the VC was created, maybe something like:

client.get_vector_collection(vc_name).blocking()

That would raise an exception if the vector collection doesn't exist.
Same comment for tests that creates a VC.

@@ -181,7 +181,21 @@ def test_backupCount_valid_values_pass(self):
name, [IndexConfig("vector", Metric.COSINE, 3)], backup_count=2, async_backup_count=2
)

def test_backupCount(self):
def test_backupCount_max_value_pass(self):
Copy link
Contributor

@yuce yuce Nov 15, 2024

Choose a reason for hiding this comment

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

Would it be possible to use lower snake case exclusively for function names, like test_backup_count_max_value_pass?
That's the expected Python style.
I've missed that in previous tests that involve the backup count.
I'll send a PR to fix them.

yuce
yuce previously approved these changes Nov 15, 2024
Copy link
Contributor

@yuce yuce 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!

@olukas olukas merged commit 0274025 into hazelcast:master Nov 15, 2024
9 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.

3 participants