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

Make Statement.setPagingSize method public #30

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

virtan
Copy link

@virtan virtan commented Jun 4, 2024

Make Statement.setPagingSize method public

Motivation:

In some cases we may need to change paging size to reduce chances of read timeouts

Modifications:

The method setPagingSize() existed but wasn't public so was inaccessible for clients

Result:

With this change it would become public and let a client to set custom paging sizes on per statement basis

@@ -131,7 +131,8 @@ extension CassandraClient {
return cass_statement_bind_collection(self.rawPointer, index, collection)
}

func setPagingSize(_ pagingSize: Int32) throws {
/// Sets the paging size of the returned paginated results.
public func setPagingSize(_ pagingSize: Int32) throws {
Copy link

Choose a reason for hiding this comment

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

Can we use Int instead here? It should be preferred for public APIs.

@virtan
Copy link
Author

virtan commented Jun 4, 2024

There is a variant of execute() method which also expects pageSize in Int32. It is public already, so changing it may introduce some backward compatibility issues.

@glbrntt
Copy link

glbrntt commented Jun 5, 2024

There is a variant of execute() method which also expects pageSize in Int32. It is public already, so changing it may introduce some backward compatibility issues.

That one should ideally be Int too, we can separately deprecate that variant of execute and add one using Int.

I'd much rather we course correct and add appropriate APIs rather than being consistent with past mistakes.

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.

2 participants