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 dynamic store connection APIs #28655

Open
wants to merge 5 commits into
base: prha/partition_key_iterators
Choose a base branch
from

Conversation

prha
Copy link
Member

@prha prha commented Mar 21, 2025

Summary & Motivation

Creates pagination connection API methods for fetching partition keys from the dynamic store (backed by the instance / event_log_storage).

How I Tested These Changes

BK

@prha prha force-pushed the prha/dynamic_store_connection branch 2 times, most recently from 55fd1f1 to b4220a7 Compare March 21, 2025 04:24
@prha prha changed the base branch from master to graphite-base/28655 March 21, 2025 04:42
@prha prha force-pushed the graphite-base/28655 branch from 4ab37c1 to 7e60a4b Compare March 21, 2025 04:42
@prha prha force-pushed the prha/dynamic_store_connection branch from b4220a7 to 1693ba5 Compare March 21, 2025 04:42
@prha prha changed the base branch from graphite-base/28655 to prha/partition_key_iterators March 21, 2025 04:43
Copy link
Member Author

prha commented Mar 21, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@prha prha force-pushed the prha/dynamic_store_connection branch from 1693ba5 to 1efb8c0 Compare March 21, 2025 18:58
@prha prha force-pushed the prha/partition_key_iterators branch from 7e60a4b to 83ef046 Compare March 21, 2025 18:58
@prha prha force-pushed the prha/dynamic_store_connection branch 2 times, most recently from d966792 to 072a617 Compare March 21, 2025 20:10
@prha prha force-pushed the prha/partition_key_iterators branch from 9495729 to cc79c5a Compare March 21, 2025 20:10
@prha prha force-pushed the prha/dynamic_store_connection branch from 072a617 to c28239b Compare March 21, 2025 21:11
@prha prha force-pushed the prha/partition_key_iterators branch from cc79c5a to 66fd0a5 Compare March 21, 2025 21:11
@prha prha force-pushed the prha/dynamic_store_connection branch from c28239b to d4c3268 Compare March 21, 2025 21:54
@prha prha force-pushed the prha/partition_key_iterators branch from 66fd0a5 to fd87fca Compare March 21, 2025 21:54
@prha prha force-pushed the prha/dynamic_store_connection branch 2 times, most recently from bf5b5f5 to c0e6838 Compare March 21, 2025 22:32
@prha prha marked this pull request as ready for review March 22, 2025 01:20
@prha prha requested review from gibsondan and OwenKephart March 24, 2025 20:34
@prha prha force-pushed the prha/dynamic_store_connection branch from c0e6838 to d88c0a5 Compare March 26, 2025 00:23
@prha prha force-pushed the prha/partition_key_iterators branch from fd87fca to 74595f5 Compare March 26, 2025 00:23
@prha prha force-pushed the prha/dynamic_store_connection branch from d88c0a5 to d81358c Compare March 26, 2025 18:58
@prha prha force-pushed the prha/partition_key_iterators branch from 74595f5 to 4d9c9df Compare March 26, 2025 18:58
Comment on lines 303 to 304
return Connection.create_from_sequence(
seq=partition_keys, limit=limit, ascending=ascending, cursor=cursor
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears that Connection.create_from_sequence() is being called, but this static method doesn't seem to be implemented in the Connection class. A search through the codebase doesn't reveal any implementation of this method, which would cause a runtime AttributeError when this code is executed.

This needs to be addressed by either:

  1. Implementing the create_from_sequence() method in the Connection class, or
  2. Modifying this code to use an existing method or directly construct the Connection object

Without this fix, the pagination functionality for dynamic partitions will fail when using cached partition keys.

GitHub Flavored Markdown

Suggested change
return Connection.create_from_sequence(
seq=partition_keys, limit=limit, ascending=ascending, cursor=cursor
return Connection.from_list(
items=partition_keys, limit=limit, ascending=ascending, cursor=cursor

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

@prha prha force-pushed the prha/dynamic_store_connection branch from 8194d38 to 3479f85 Compare March 28, 2025 00:50
@prha prha force-pushed the prha/partition_key_iterators branch from 15af281 to 4d30519 Compare March 28, 2025 05:41
@prha prha force-pushed the prha/dynamic_store_connection branch from 3479f85 to 60ace3b Compare March 28, 2025 05:41
Copy link
Member

@gibsondan gibsondan left a comment

Choose a reason for hiding this comment

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

very exciting

Copy link
Contributor

@OwenKephart OwenKephart left a comment

Choose a reason for hiding this comment

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

note comment

@prha prha force-pushed the prha/dynamic_store_connection branch from 60ace3b to 3f91631 Compare April 1, 2025 22:30
@prha prha force-pushed the prha/partition_key_iterators branch from 4d30519 to bc0578f Compare April 1, 2025 22:30
@prha prha force-pushed the prha/dynamic_store_connection branch from 3f91631 to 596cbbd Compare April 1, 2025 22:34
@prha prha force-pushed the prha/partition_key_iterators branch from 1cd8d9b to 521c5cf Compare April 1, 2025 23:15
@prha prha force-pushed the prha/dynamic_store_connection branch from 596cbbd to 6713344 Compare April 1, 2025 23:15
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