Skip to content
This repository has been archived by the owner on Jan 15, 2020. It is now read-only.

AsyncIO backend #46

Closed
wants to merge 8 commits into from
Closed

AsyncIO backend #46

wants to merge 8 commits into from

Conversation

kxepal
Copy link

@kxepal kxepal commented Feb 8, 2016

No description provided.

@kxepal
Copy link
Author

kxepal commented Feb 8, 2016

Quite basic, but functional version. Tests have to be moved into package since otherwise 2.7 will raise assertions about yield from syntax, so we have to be more precise on what we run.

There are left some wtf I would like to get rid (private flags and state thing), but need to sleep over with current implementation to figure out how to do that better. Meanwhile, any comments are welcome!

@isagalaev
Copy link
Owner

Thanks! I should be able to find time to look at it on the week.

Ethan and others added 5 commits February 10, 2016 11:36
This unifies (a bit) yajl* backends implementations, hiding
specifics behind common interface.

Suddenly, we cannot make them share common bits since they still
different in details like order of the arguments.
Unittest and generators are not the best friends, so that's how a missed
metaclass definition prevented tests from run hiding dozens of
the issues.
Two general issues were addressed:
- Handle incomplete JSON data read
- Handle UTF-8 data split during read
@kxepal
Copy link
Author

kxepal commented Feb 16, 2016

@isagalaev I'd updated PR, including initial implementation cleanup and support for yajl backends reuse. Left them unsquashed to let you see what exactly had changed.

@ethanfrey If you'd like to test asyncio backend with yajl - it's possible now (:

@kxepal
Copy link
Author

kxepal commented Feb 28, 2016

Hi @isagalaev ! Any news on this?

@isagalaev
Copy link
Owner

Not yet, sorry :-( Things keep getting in the way.

@luigiberrettini
Copy link

luigiberrettini commented Jul 7, 2017

Sorry @kxepal I do not understand the README code showing hot to use the asyncio backend with the yajl* ones:

import ijson.backends.asyncio as ijson
from ijson.backends import yajl2

async def go():
    resp = await aiohttp.get('http://localhost:5984')
    items = ijson.items(resp.content, '')
    async for item for ijson.items(resp.content, '', yajl_backend=yajl2)

I do not have much experience with Python therefore I might be wrong, but I think it should be changed to:

import ijson.backends.asyncio as ijson
from ijson.backends import yajl2

async def go():
    resp = await aiohttp.get('http://localhost:5984')
    async for item in ijson.items(resp.content, '', yajl_backend=yajl2)

Thanks in advance for any help you can provide!

@kxepal
Copy link
Author

kxepal commented Jul 7, 2017

@luigiberrettini Nice found! Yeah, that's a typo indeed.

Oh, there are conflicts now. I'll fix that with solving them. @isagalaev are you still ok with the patch?

@luigiberrettini
Copy link

Thanks!
I merged the 3 PR in my fork and there wasn't a conflict 🤔

@carver carver mentioned this pull request Dec 12, 2017
@rtobar
Copy link
Contributor

rtobar commented Jul 18, 2019

Hi,

I know it's been a long time, but you may be interested to know that maintenance of this project has been handed over. ICRAR/ijson is hosting the code that gets published into PyPI, and a new version (2.4) was actually released not too long ago.

Would you be interested in getting this code merged (again) and published in a new version of ijson? A few things have changed since the last time this happened, so a few adjustments would certainly be required. I tried locally to merge, and while it works there are a few warnings/errors with python 3.6 and 3.7 that need to be taken care of. There's also a new C backend that has the potential of being ported to be async too, but that would be a separate piece of work probably as I imagine it would be much harder to come up with.

@kxepal
Copy link
Author

kxepal commented Jul 18, 2019

Hi! Actually, I'm closing this PR and now consider it wrong (thanks @isagalaev for not accepting it). The better solution would be to use sans-io way to build a bridge between asyncio and ijson - this would require a few code on user side while wouldn't couple this library with specific framework. Asyncio is not the only one: there are trio, curio and others and making special module for each support is not a right way to go.

@kxepal kxepal closed this Jul 18, 2019
@isagalaev
Copy link
Owner

In fact, my idle thinking on the subject over the past few years led me to believe that the main ijsons API — where you consume an iterator — is the wrong model because it couples parsing with one concrete way of doing IO. I.e., upon calling __next__() ijson itself decides if it needs more data and requests it. It should be decoupled: ijson should only parse whatever (potentially incomplete) buffer it's given and leave IO to the user. Of course, that doesn't preclude the library to have an out of the box high level API wrapping all kinds of IO, but the important point is that the core parser should allow for it.

Anyway, it's thankfully out of my hands by now, I'm just dumping some raw thoughts in case anyone's interested.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants