Skip to content

Conversation

@analogrelay
Copy link
Member

@analogrelay analogrelay commented Nov 14, 2025

This PR ports over the initial C bindings, contributed by a partner team, to this repository.

The bindings are defined in the azure_data_cosmos_native crate, which builds libazurecosmos.a and libazurecosmos.so (and others on other platforms). That project also creates the azurecosmos.h header file.

@github-actions github-actions bot added Azure.Core The azure_core crate Cosmos The azure_cosmos crate labels Nov 14, 2025
@github-actions
Copy link

github-actions bot commented Nov 14, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure_data_cosmos

@analogrelay analogrelay force-pushed the ashleyst/port-c-bindings branch from 4393c1b to aeaba92 Compare November 18, 2025 00:46
@analogrelay analogrelay changed the title [WIP] Initial C Bindings for Cosmos DB SDK for Rust Initial C Bindings for Cosmos DB SDK for Rust Nov 18, 2025
@analogrelay analogrelay marked this pull request as ready for review November 20, 2025 00:03
@analogrelay analogrelay requested a review from a team as a code owner November 20, 2025 00:03
Copilot AI review requested due to automatic review settings November 20, 2025 00:03
Copilot finished reviewing on behalf of analogrelay November 20, 2025 00:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces C bindings for the Azure Cosmos DB SDK for Rust, allowing the SDK to be used from C and other languages. Key changes include:

  • New FFI layer with runtime context, call context, and error handling infrastructure
  • C bindings for CosmosClient, DatabaseClient, and ContainerClient
  • Support for CRUD operations, queries, and resource management
  • Auto-generated C header file (azurecosmos.h)
  • Comprehensive C test suite covering CRUD operations, error handling, and memory management

Reviewed Changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/string.rs String utilities and FFI string conversion helpers
src/runtime/tokio.rs Tokio runtime wrapper for async operations
src/runtime/mod.rs Runtime context FFI functions
src/options/mod.rs Empty option structs for future extensibility
src/lib.rs Main library entry, version function, and tracing setup
src/error.rs Error types, codes, and conversion from Azure SDK errors
src/context.rs Call context for FFI operations and helper macros
src/clients/mod.rs Client module exports
src/clients/database_client.rs Database operations FFI functions
src/clients/cosmos_client.rs Cosmos client creation and database operations
src/clients/container_client.rs Container and item operations FFI functions
include/azurecosmos.h Auto-generated C header file
c_tests/*.c C test suite for FFI functionality
build.rs cbindgen configuration for header generation
Cargo.toml Dependencies and features for native bindings
CMakeLists.txt CMake build configuration for C tests
Comments suppressed due to low confidence (1)

sdk/cosmos/azure_data_cosmos_native/src/string.rs:37

  • The safe_cstring_new function calls expect() which can panic, but this panic behavior is not documented. Add a # Panics section to the function documentation explaining when this function panics (when the string contains interior NUL bytes).


#[repr(C)]
pub struct DeleteDatabaseOptions {
// Placeholder for future read database options
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

Corrected comment from 'read database options' to 'delete database options'.

Suggested change
// Placeholder for future read database options
// Placeholder for future delete database options

Copilot uses AI. Check for mistakes.

#[repr(C)]
pub struct DeleteContainerOptions {
// Placeholder for future read container options
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

Corrected comment from 'read container options' to 'delete container options'.

Suggested change
// Placeholder for future read container options
// Placeholder for future delete container options

Copilot uses AI. Check for mistakes.
Comment on lines +166 to +167
let pk = partition_key.to_string();
container.replace_item(pk, item_id, raw_value, None).await?;
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The partition_key is already converted to String on line 161. This .to_string() call creates an unnecessary allocation. Use partition_key directly instead of creating pk.

Suggested change
let pk = partition_key.to_string();
container.replace_item(pk, item_id, raw_value, None).await?;
container.replace_item(partition_key, item_id, raw_value, None).await?;

Copilot uses AI. Check for mistakes.
}

// Heap-allocated context
cosmos_call_context *ctx = cosmos_call_context_create(runtime, false);
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The cosmos_call_context_create function expects a pointer to CallContextOptions as its second parameter (const struct cosmos_call_context_options *options), but false (a boolean literal) is being passed instead. This should either be NULL or a pointer to a properly initialized cosmos_call_context_options struct with include_error_details set to false.

Suggested change
cosmos_call_context *ctx = cosmos_call_context_create(runtime, false);
struct cosmos_call_context_options options = { 0 };
options.include_error_details = false;
cosmos_call_context *ctx = cosmos_call_context_create(runtime, &options);

Copilot uses AI. Check for mistakes.
# cSpell:ignore cosmosctest CRATETYPES endforeach

project(cosmosctest C)
cmake_minimum_required(VERSION 4.1)
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

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

The project() command should be called before cmake_minimum_required() according to CMake best practices. While this code moves project() after cmake_minimum_required() (which is correct), the version 4.1 appears incorrect as CMake versions follow a 3.x numbering scheme. This should likely be VERSION 3.1 or higher.

Suggested change
cmake_minimum_required(VERSION 4.1)
cmake_minimum_required(VERSION 3.15)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure.Core The azure_core crate Cosmos The azure_cosmos crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant