Skip to content

[object_cache] add application id to object cache#346

Open
jinzhejz wants to merge 1 commit intoxflops:mainfrom
jinzhejz:cache_add_applicatoin_id
Open

[object_cache] add application id to object cache#346
jinzhejz wants to merge 1 commit intoxflops:mainfrom
jinzhejz:cache_add_applicatoin_id

Conversation

@jinzhejz
Copy link

@jinzhejz jinzhejz commented Feb 6, 2026

No description provided.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @jinzhejz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the object caching system by introducing an application_id to create a more structured and isolated storage hierarchy. This change impacts how objects are stored on disk, how they are referenced in the Python SDK, and how the underlying Rust cache service manages data. It also provides a standalone executable for the cache and expands testing to ensure robustness and correct behavior with the new organizational paradigm.

Highlights

  • Application ID Integration: The object cache now incorporates an application_id into its key structure, changing the storage format to {application_id}/{session_id}/{object_id}.arrow for better organization and isolation.
  • Python SDK Updates: The put_object function in the Python SDK has been updated to require application_id as its first parameter, and related calls throughout the codebase have been adjusted accordingly.
  • Rust Cache Implementation Changes: The core Rust cache logic has been refactored to handle the new application_id in file paths, key parsing, and directory creation, ensuring a hierarchical storage structure.
  • New Executable and Configuration: A new flame-object-cache binary has been added, allowing the cache to run as a standalone service. It supports configuration via CLI arguments, environment variables, or a YAML file, and includes enhanced logging options.
  • Expanded Test Coverage: New unit and integration tests have been introduced for the Rust object cache, covering put, get, update, delete, persistence, and gRPC interactions with the new application_id parameter.
Changelog
  • Cargo.lock
    • Added serde_yaml and tempfile dependencies.
  • docs/designs/RFE284-flmrun/runpy-enhancement-summary.md
    • Updated put_object call to include application_id.
  • docs/designs/RFE284-flmrun/runpy-enhancements.md
    • Updated put_object call to include application_id.
  • docs/designs/RFE318-cache/FS.md
    • Updated get_flight_info request path to include application_id.
    • Modified put_object signature and description to include application_id.
    • Updated storage organization and key format to include application_id.
    • Adjusted do_put and do_get algorithms to handle application_id.
    • Updated input validation to include application_id.
    • Modified examples to use application_id in put_object calls and key formats.
  • docs/designs/RFE318-cache/IMPLEMENTATION.md
    • Updated ObjectRef key format to include application_id.
    • Modified Python SDK put_object signature and description.
    • Updated storage filename format to include application_id.
    • Modified Python SDK example to include application_id.
  • docs/designs/RFE323-runner-v2/FS.md
    • Updated put_object calls to include application_id.
  • e2e/src/e2e/helpers.py
    • Updated put_object calls to include app_name as application_id.
  • e2e/tests/test_cache.py
    • Updated test cases to include application_id in put_object calls and key assertions.
  • object_cache/Cargo.toml
    • Added [[bin]] section for flame-object-cache executable.
    • Added serde_yaml dependency.
    • Added tempfile as a dev-dependency.
  • object_cache/README.md
    • Updated key-based organization description to include application_id.
    • Added RUST_LOG environment variable documentation for logging configuration.
    • Updated Python SDK example to include application_id.
    • Updated storage directory structure example to include application_id.
    • Added detailed logging configuration section.
    • Added note about viewing cache logs with debug level in Docker Compose.
  • object_cache/src/cache.rs
    • Modified load_session_objects to accept application_id.
    • Updated key formatting in load_session_objects to include application_id.
    • Changed load_objects_from_disk to iterate through application directories.
    • Modified put and put_with_id functions to accept application_id and create application/session directory structure.
    • Updated get and get_if_exists to parse application_id from the key.
    • Modified update to parse application_id from the key and use it in the file path.
    • Modified delete to accept application_id and delete the application/session directory.
    • Updated retain logic in delete to use the full application_id/session_id prefix.
    • Renamed extract_session_and_object_id to extract_application_session_and_object_id and updated its logic to extract application_id from FlightDescriptor path or app_metadata.
    • Modified collect_batches_from_stream to collect application_id.
    • Updated handle_put_action to parse application_id from the action body.
    • Updated handle_delete_action to parse application_id from the action body and enforce its presence.
    • Updated key format comments in get_flight_info and do_get.
    • Modified do_put to pass application_id to put_with_id.
    • Added new test cases for put_and_get, put_with_id, update, delete, list_all, get_nonexistent_object, persistence, multiple_applications_and_sessions, and invalid_key_format.
  • object_cache/src/main.rs
    • Added main executable for flame-object-cache, including CLI argument parsing, YAML configuration loading, and environment variable handling for cache settings.
  • object_cache/tests/integration_test.rs
    • Added gRPC integration tests for do_put, do_get, get_flight_info, list_flights, do_action (PUT/DELETE), and get_schema, verifying the new application_id handling.
  • sdk/python/src/flamepy/agent/client.py
    • Updated put_object call to include name as application_id.
  • sdk/python/src/flamepy/core/cache.py
    • Updated ObjectRef key format description.
    • Modified put_object signature to include application_id and updated its implementation to use the new key format and directory structure.
    • Modified update_object to validate the new 3-part key format.
  • sdk/python/src/flamepy/runner/runner.py
    • Updated put_object call to include app as application_id.
  • sdk/python/src/flamepy/runner/runpy.py
    • Updated put_object call to include application_id from self._ssn_ctx.application.name.
Activity
  • The author jinzhejz initiated this pull request to introduce application_id into the object cache.
  • The changes span across multiple components, including the core Rust cache implementation, Python SDK, and documentation, indicating a thorough and integrated update.
  • New Rust unit tests have been added to object_cache/src/cache.rs to validate the new application_id logic, covering various scenarios like persistence and multi-application/session handling.
  • New gRPC integration tests have been added in object_cache/tests/integration_test.rs to verify the functionality of do_put, do_get, get_flight_info, list_flights, and do_action with the new application_id.
  • The project's build configuration was updated in object_cache/Cargo.toml to include new dependencies and define a standalone executable for the object cache, suggesting an expansion of deployment options.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces application_id to the object cache keying scheme, changing it to {application_id}/{session_id}/{object_id}. This is a significant and necessary enhancement for multi-application environments. The changes are comprehensive, touching the cache server, Python SDK, documentation, and tests. The addition of a standalone binary for the object cache and extensive integration tests are particularly valuable.

My review identifies a couple of issues in the Rust implementation related to backward compatibility. The attempt to support legacy 2-part keys in do_put seems to be broken, and the handling of legacy formats in do_action is confusing. Addressing these will improve the robustness and clarity of the implementation.

Comment on lines +481 to 485
} else if parts.len() == 2 {
// Legacy format: session_id/object_id (for backward compatibility)
*session_id = Some(parts[0].to_string());
*object_id = Some(parts[1].to_string());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The backward compatibility logic for 2-part paths (session_id/object_id) in do_put operations appears to be incomplete. When a legacy client provides a 2-part path, application_id is not set. The subsequent logic in collect_batches_from_stream requires an application_id and will fail, as the Python client does not seem to set the app_metadata fallback.

This means the do_put operation will fail for legacy clients, despite the code suggesting backward compatibility.

To fix this, you could consider using a default application_id for legacy paths or making application_id optional in put_with_id for this specific case. If backward compatibility is not a goal for do_put, this code path should be removed to avoid confusion.

Comment on lines +680 to +685
// Legacy format: only session_id (for backward compatibility)
// Try to infer application_id from existing objects
return Err(FlameError::InvalidState(
"DELETE action requires application_id/session_id format".to_string(),
));
};
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The comment on line 680 suggests backward compatibility for the legacy DELETE action format, but the implementation immediately returns an error. Furthermore, the comment on line 681, "Try to infer application_id from existing objects", describes logic that is not implemented.

This is misleading. If the intention is to break compatibility for the DELETE action, the comments should be removed to reflect the actual behavior. If backward compatibility is desired, the logic to infer application_id would need to be implemented.

@jinzhejz jinzhejz force-pushed the cache_add_applicatoin_id branch from 582b1fa to 3498cba Compare February 9, 2026 01:32
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.

1 participant