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

Wrap tiledb_handle_load_array_schema_request. #329

Merged
merged 7 commits into from
Jul 18, 2024

Conversation

shaunrd0
Copy link
Contributor

This wraps tiledb_handle_load_array_schema_request added in TileDB-Inc/TileDB#4399. This binding is required for https://github.com/TileDB-Inc/TileDB-Cloud-REST/pull/4686.

@shaunrd0 shaunrd0 marked this pull request as ready for review July 16, 2024 13:28
serialize.go Outdated
@@ -514,6 +514,23 @@ func DeserializeQueryAndArray(context *Context, buffer *Buffer, serializationTyp
return array, query, nil
}

// HandleLoadArraySchemaRequest Pass the array and serialized request to core which returns the serialized response.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be made more descriptive? Most functions in this package pass something to core, so that doesn’t really illuminate much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

serialize.go Outdated
@@ -514,6 +514,23 @@ func DeserializeQueryAndArray(context *Context, buffer *Buffer, serializationTyp
return array, query, nil
}

// HandleLoadArraySchemaRequest Pass the array and serialized request to core which returns the serialized response.
func HandleLoadArraySchemaRequest(array *Array, serializationType SerializationType, request *Buffer) (*Buffer, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at a few of our functions above, most of them seem to put the request in front of the serialization type in the parameter list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

serialize.go Outdated
}

runtime.KeepAlive(request)
runtime.KeepAlive(response)
Copy link
Contributor

Choose a reason for hiding this comment

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

This really isn't necessary, since the buffer is returned, but isn't harmful either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed 👍

@ypatia ypatia merged commit a26aa12 into master Jul 18, 2024
7 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.

5 participants