Updates docs to handle async persister case#496
Merged
elijahbenizzy merged 2 commits intomainfrom Jan 13, 2025
Merged
Conversation
- Clarifies the concepts to be more relevant to Burr - Adds docs for async persister in the references
Contributor
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 7e0792f in 13 seconds
More details
- Looked at
183lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. docs/concepts/sync-vs-async.rst:10
- Draft comment:
Typo: 'avaialble' should be 'available'.
1. Use the `async` interfaces when you have I/O-heavy applications that require horizontal scaling, and have available asynchronous APIs (E.G. async LLM APIs)
- Reason this comment was not posted:
Confidence changes required:10%
The word 'avaialble' is misspelled and should be corrected to 'available'.
2. docs/concepts/sync-vs-async.rst:43
- Draft comment:
Typo: 'suports' should be 'supports'.
are missing a specific implementation). Furthermore, Burr supports the following APIs for both synchronous/asynchronous interfaces:
- Reason this comment was not posted:
Confidence changes required:10%
The word 'suports' is misspelled and should be corrected to 'supports'.
3. docs/concepts/sync-vs-async.rst:53
- Draft comment:
Typo: 'bellow' should be 'below'.
legacy code, ...) and we give some options to handle that. The table below shows the
- Reason this comment was not posted:
Confidence changes required:10%
The word 'bellow' is misspelled and should be corrected to 'below'.
Workflow ID: wflow_STStILKNcOgfuj1E
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
A preview of 5c18047 is uploaded and can be seen here: ✨ https://burr.dagworks.io/pull/496 ✨ Changes may take a few minutes to propagate. Since this is a preview of production, content with |
skrawcz
reviewed
Jan 13, 2025
skrawcz
reviewed
Jan 13, 2025
skrawcz
reviewed
Jan 13, 2025
skrawcz
reviewed
Jan 13, 2025
skrawcz
reviewed
Jan 13, 2025
skrawcz
reviewed
Jan 13, 2025
skrawcz
reviewed
Jan 13, 2025
skrawcz
approved these changes
Jan 13, 2025
Contributor
skrawcz
left a comment
There was a problem hiding this comment.
added more specificity - since the language is ambiguous by what "application" is being referred to...
Co-authored-by: Stefan Krawczyk <stefan@dagworks.io>
This file contains hidden or 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
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.
Important
Updates documentation to clarify sync vs async usage and add async persister references in Burr.
sync-vs-async.rst.persister.rst.AsyncBaseStatePersister,AsyncBaseStateLoader, andAsyncBaseStateSaverclasses.index.rstto be more relevant to Burr.This description was created by
for 7e0792f. It will automatically update as commits are pushed.