-
Notifications
You must be signed in to change notification settings - Fork 98
Support for CSV formatted results of subscriptions #981
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
Conversation
| # retry logic for HTTP requests, but does not handle errors | ||
| # during streaming. We may want to consider a retry decorator | ||
| # for this entire method a la stamina: | ||
| # https://github.com/hynek/stamina. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that our model for retrying requests doesn't cover streaming cases.
Retrying can wait until after 2.1.0.
|
Thanks. Using this via Are we clear in the documentation about how to get the output into a CSV file? I think we have users for which that might not be obvious. |
| async with Session() as session: | ||
| client = SubscriptionsClient(session, base_url=TEST_URL) | ||
| results = [res async for res in client.get_results_csv("42")] | ||
| import csv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised yapf doesn't care about this import's placement.
|
@guyon-planet in the future, let me merge these. I have a couple changes to make yet. |
|
@sgillies, sorry about that |
Resolves #933. The big choice here is to make CSV format access its own method instead of a flag on the existing method. I feel like this is warranted by the API split between JSON (paged) and CSV (not paged).
Sanity check @kevinlacaille @cpaulik ?