Formally adds state persistence #73
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
elijahbenizzy
added a commit
that referenced
this pull request
Mar 15, 2024
…placed by persistence in #73
elijahbenizzy
added a commit
that referenced
this pull request
Mar 15, 2024
…placed by persistence in #73
skrawcz
force-pushed
the
add_persister
branch
3 times, most recently
from
f157f50
to
ade5e91
Compare
elijahbenizzy
requested changes
Mar 16, 2024
There was a problem hiding this comment.
OK, big things:
- Persistence handling failure is confusing me quite a bit, and we don't actually use it
- Sequence IDs are going to start at 1 (unless I'm mistaken)
- API is confusing me -- maybe docs will clear this up? We have
trackers,persisters,initialize_from, etc... and its far from obvious which does which. Might need to revisit. - Docs, remove a few TODOs prior to merge
Merged
elijahbenizzy
approved these changes
Mar 18, 2024
There was a problem hiding this comment.
Merged in #75 -- this has all the changes we discussed. Also adds partition key to UI.
This does a few things: 1. adds interface for persister and SQLLite implementation. 2. pushes app_id, and partition_key through to App and hooks for (1) 3. moves sequence ID incrementing to the start of a step to avoid bad restarts; we were only incrementing after saving which was bad. 4. shows API in counter example with changes to builder required to facilitate the above. TODOs: - test updates - docs - modifying tracker
So the return of load was getting out of hand. Added TypedDict response. I feel like that's a bit easier to extend. Also decided that save should have a status field. So the persister can make a choice whether to save it or not, and then likewise when loading someone can determine what behavior they want. Otherwise possible edge cases / bugs to iron out: - async/streaming -- not tested - sequence id needs to be done prior to the step - also reloading from an error nuking `__prior_step` means that it isn't set until the third iteration weirdly. Haven't had a chance to dig.
I think this should minimally cover things. I also refactored things to be more readable.
This changes the with_tracker signature to check if the thing being passed has hooks and can be added as a lifecycle adapter. Otherwise this cleans up some imports and stubs out the functions to make the local tracking client also a persister.
This mostly adds TODOs but it sets up the structure.
This shows how to quickly implement something that will create "spans" in the burr UI for the given framework.
Following changes: 1. Makes the state the source of truth in applications 2. Adds error-checking on the initialize API 3. Breaks up the persistor API to multiple pieces to allow for the tracking client to use it 4. Adds misc docs/cleans up
This adds the partition key in a new metadata.json file in the application directory. This defaults to None (valid) if empty, so we're backwards compatible.
elijahbenizzy
force-pushed
the
add_persister
branch
from
68925dd
to
a5bc532
Compare
elijahbenizzy
force-pushed
the
add_persister
branch
from
a5bc532
to
5877f78
Compare
Closed
elijahbenizzy
added a commit
that referenced
this pull request
Mar 18, 2024
…placed by persistence in #73
elijahbenizzy
added a commit
that referenced
this pull request
Mar 19, 2024
…placed by persistence in #73
elijahbenizzy
added a commit
that referenced
this pull request
Mar 19, 2024
…placed by persistence in #73
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does a few things: