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.

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

Leave staggered orders when exiting from dexbot-cli #148

Conversation

bitphage
Copy link
Collaborator

  1. Change how INT/HUP/TERM signals are being handled. Instead of
    invoking worker.stop job, define a separate worker.clean_exit job which
    actually do the same things as stop(), but instead of cancel_all() it
    calls clean_exit() method. Thus, each strategy can separately define how
    it should handle exit signals.

  2. In staggered_orders.py strategy define clean_exit() method which
    doesn't cancel orders but leaves them.

  3. Because of (2), staggered_orders.py at the every worker startup
    should not try to place any orders from database. Since previous run
    things on the marked may drastically change and just placing old orders
    is just an error. So, just recalculate everything after worker startup
    and place newly calculated orders.

Closes: #137

@ihaywood3
Copy link

in this case, if you did want to cancel orders and leave a market, how would you do so? yes you could manually delete orders, but for staggered orders that could take a long time.
also, why do you want to turn the CLI on and off: isn't it for 24/7 operation?
one option would be different signals for different things: say SIGUSR1 for "exit and leave orders on market"

@bitphage
Copy link
Collaborator Author

#137 says "Staggered Orders should be left on the books when exiting dexbot-cli with ctrl+c", I implemented exactly this.

For leaving a market and cancelling all orders I think better to have a CLI option for that, example: dexbot-cli --cancel workername instead of playing with different signals.

@MarkoPaasila
Copy link
Collaborator

There are reasons to exit dexbot-cli. One is upgrading the software. Sometimes you need reboot. Also if you need to kill dexbot because of a bug, it isn't convenient if it cancels all orders at restart.

@bitphage bitphage force-pushed the 137-prevent-staggered-orders-cancel-on-ctrl-c branch 2 times, most recently from 6e808c0 to b8c8b18 Compare May 29, 2018 15:11
@bitphage
Copy link
Collaborator Author

OK, I finally changed a PR to not touch the previous logic of orders placement. I also got rid of code-duplicating clean_exit() and made it a wrapper of stop(), and added appropriate handling in stop().

1. Change how INT/HUP/TERM signals are being handled. Instead of
invoking worker.stop job, define a separate worker.clean_exit job which
is a wrapper on stop().

2. Add clean_exit kwarg to worker stop() method. Thus, each strategy can
separately define a handler for clean_exit.

3. In staggered_orders.py strategy define clean_exit() method which
doesn't cancel orders but leaves them.

Closes: Codaone#137
@bitphage
Copy link
Collaborator Author

I moved the fix to #161

@bitphage bitphage closed this May 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants