CityOfZion / neo-python

Python Node and SDK for the NEO 2.x blockchain. For NEO 3.x go to our successor project neo-mamba
https://neo-python.readthedocs.io/en/latest/
MIT License
313 stars 189 forks source link

np-api-server auto exit #1021

Closed jseagrave21 closed 4 years ago

jseagrave21 commented 4 years ago

What current issue(s) does this address, or what feature is it adding? Partially addresses https://github.com/CityOfZion/neo-python/issues/1020, specifically issues 1-3. Now, the program will automatically exit whenever an issue arises when opening a wallet or setting minpeers/maxpeers.

I haven't figured out how to solve issue 4 from https://github.com/CityOfZion/neo-python/issues/1020.

(-edit-) All issues from https://github.com/CityOfZion/neo-python/issues/1020 have been addressed thanks to input from @ixje

How did you solve this problem? I replaced applicable instances of return with raise SystemExit which had precedence for issues setting port-rpc and port-rest.

How did you make sure your solution works? Manual testing

Are there any special changes in the code that we should be aware of? No.

Please check the following, if applicable:

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 85.313% when pulling 510a1ccc1ab161a0197ef9f74f4c373e5f4aec0a on jseagrave21:np-api-server-auto-exit into 70ded9e77372752355e0298c6b55d6ee06e28d04 on CityOfZion:development.

ixje commented 4 years ago

Can you please include the fix from this https://github.com/CityOfZion/neo-python/issues/1020#issuecomment-533987238 into your PR?

ixje commented 4 years ago

Issue 4 can be solved by changing https://github.com/CityOfZion/neo-python/blob/5fe7d51ef14614ff1cd0681f8fe2cb3e2cb76557/neo/bin/api_server.py#L272 to

runner = web.AppRunner(api_server_rpc.app, handle_signals=True)

Note: it will not shutdown instantly but will do it asynchronously. We can't seem to control that. It's up to the aiohttp package when it decides to run the handlers shrug

-edit- don't forget to also apply the above fix to line 285

jseagrave21 commented 4 years ago

Can you please include the fix from this #1020 (comment) into your PR? don't forget to also apply the above fix to line 285

@ixje will do. Thank you!

ixje commented 4 years ago

💯