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.

Sign up for GitHub

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

Jump to bottom

Consider moving to asyncio before the library gets too large #18

Closed
devraj opened this issue Dec 6, 2023 · 2 comments
Closed

Consider moving to asyncio before the library gets too large #18

devraj opened this issue Dec 6, 2023 · 2 comments
Assignees
@devraj

Comments

@devraj
Copy link
Member

devraj commented Dec 6, 2023

We're getting to a stage where the library design and patterns are maturing. Tests are running consistently and passing as expected.

It would thus be time to full flesh out the entire library before we move into some enterprise features and use this in integrations and products.

Most of Anomaly's projects have moved to using asyncio and it would thus be unwise to not make this library have asyncio support. We would have to consider:
@devraj devraj self-assigned this Dec 6, 2023
This was referenced Dec 6, 2023
Open
Open
devraj added a commit that referenced this issue Dec 18, 2023
@devraj
chore: move to concurrency
cc73e4e 
this is our attempt to move to concurrency before the library goes out of hand
there are a few exception at the moment around typer not being able to support
async/await we can work around that when they support it

#18
@devraj
Copy link
Member Author

devraj commented Dec 18, 2023

See also streaming responses for long poll calls to the API.

devraj added a commit that referenced this issue Dec 18, 2023
@devraj
fix: basic lifecycle of asyncio working
86918f8 
moved to explicitly closing the client upon each requestREFS #18
devraj added a commit that referenced this issue Dec 18, 2023
@devraj
refactor: all tests are now async
baaf841 
pending running a full test this changes all the test calls to be
async, we recomend merging this branch into the dto-implementation
branch and let the tests run against the server to see if the
tests pass

REFS #18
@devraj devraj mentioned this issue Dec 18, 2023
Merged
devraj added a commit that referenced this issue Dec 18, 2023
@devraj
refactor: move configuration to package root
0c0c959 
moves the environment variable configuration to the root of the package
clean up various imports post asyncio workaround, see previous commit
for details on how typer asyncio works

REFS #18 #13
@devraj devraj mentioned this issue Dec 18, 2023
Merged
@devraj
Copy link
Member Author

devraj commented Dec 18, 2023

This in principle is complete, however see commit 620b652 as typer officially does not support async yet. There's an ongoing discussion fastapi/typer#88 and the Roadmap fastapi/typer#678 discusses this as well.

For now we have a wrapper class adapted from @gilcu2's comment on the thread.

This issue is being closed for now, we should refactor the cli implementation as typer introduces support for concurrency.

@devraj devraj closed this as completed Dec 18, 2023
@devraj devraj mentioned this issue Dec 19, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@devraj devraj

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@devraj