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

Updates docs to handle async persister case #496

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

elijahbenizzy
Copy link
Contributor

@elijahbenizzy elijahbenizzy commented Jan 12, 2025

  • Clarifies the concepts to be more relevant to Burr
  • Adds docs for async persister in the references

Important

Updates documentation to clarify sync vs async usage and add async persister references in Burr.

  • Concepts:
    • Clarifies sync vs async application usage in sync-vs-async.rst.
    • Adds a comparison section and a table of supported sync/async combinations.
  • References:
    • Adds async persister documentation in persister.rst.
    • Documents AsyncBaseStatePersister, AsyncBaseStateLoader, and AsyncBaseStateSaver classes.
  • Misc:
    • Fixes order of topics in index.rst to be more relevant to Burr.

This description was created by Ellipsis for 7e0792f. It will automatically update as commits are pushed.

- Clarifies the concepts to be more relevant to Burr
- Adds docs for async persister in the references
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 7e0792f in 13 seconds

More details
  • Looked at 183 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted 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.

Copy link

github-actions bot commented Jan 12, 2025

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 draft: true will not be rendered. The source is here: https://github.com/DAGWorks-Inc/burr/tree/gh-pages/pull/496/

Copy link
Contributor

@skrawcz skrawcz left a comment

Choose a reason for hiding this comment

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

added more specificity - since the language is ambiguous by what "application" is being referred to...

@elijahbenizzy elijahbenizzy merged commit d89175e into main Jan 13, 2025
11 checks passed
@elijahbenizzy elijahbenizzy deleted the async-documentation-improvements branch January 13, 2025 02:01
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.

2 participants