expo-community / expo-server-sdk-python

Exponent Server SDK

MIT License

143 stars 42 forks source link

Implementation of asynchronous push client #34

Open shpaker opened 3 years ago

shpaker commented 3 years ago

julianpacker commented 3 years ago

Hey, sorry about the delay reviewing. Will be reviewing within a day or 2.

Did you pull this locally and test this and confirm both the synchronous and async method work? There's no tests in the repo at the moment.

And from the looks of it you also just cleaned up the synchronous method as well, but no functional change?

In addition I think I understand what you're doing but if you can provide a slightly larger description that would be appreciated!

SibiAkkash commented 3 years ago

39 This might help in the testing process

ahmedalshamary commented 2 years ago

Hi, was wondering about the status of this merge request and if I could help in any way to get this merged in.

julianpacker commented 2 years ago

Hi, I was looking for reasoning of this change, and review / testing to make sure it wouldn't break current applications. Is this a change you are interested in?

If so, do you mind explaining what this change is doing?

Will also need to resolve current merge conflicts here but if everything's good, I'm able to merge / redeploy

shpaker commented 2 years ago

Hi! Now, i am not working with expo-sdk and i did this merge request for tasks for one startup-company (not active at current time). This patch allows to use this library with asynchronous code (AsyncClient class) and without any blocking https-requests. It’s necessary to resolve merge conflicts, test this and merge this into the “master”. I tried to do this yesterday but i’m afraid to break anything in code because i don’t use this now + tests are absent.

julianpacker commented 1 year ago

Okay looks like this is valuable, but would need to be tested. When I have some free time, I'll download this and test it works ok with my app before i can approve this