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

improved streamOrders #687 #780

Merged
merged 5 commits into from
Jan 15, 2019
Merged

improved streamOrders #687 #780

merged 5 commits into from
Jan 15, 2019

Conversation

rsercano
Copy link
Collaborator

@rsercano rsercano commented Jan 7, 2019

Hello again,

this PR fixes #687, I was trying to work on new order types but since there're lots of struggles there I wanted to focus something else, as requested by @offerm I've implemented all the following changes;

  • a. allow it to start before xud. streamorders should wait for xud to start and not issue an error. It is OK to issue a warn/info for waiting for XUD.
  • b. print a line upon connection
  • c. upon termination of xud or any other RPC error - reconnect to xud (wait if xud is not available.
  • d. upon connection to xud - print existing orders.

In a technical overview; I've implemented those by calling GetInfo to ensure connection before subscribing anything and if there's no xud node running then I'm trying to reconnect in 3 seconds (let me know if you want to adjust time) This seemed to be the easiest way to me to implement such logic. Btw. error messages are not being printed every 3 seconds it's only being printed at first.

@rsercano rsercano added the command line (CLI) Relating to the command line interface tools label Jan 7, 2019
@rsercano rsercano self-assigned this Jan 7, 2019
@ghost ghost added the in progress label Jan 7, 2019
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
@moshababo
Copy link
Collaborator

@rsercano Thanks! Let's wait for @offerm review, since he asked for this feature.

lib/cli/commands/subscribeorders.ts Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
lib/cli/commands/subscribeorders.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@sangaman sangaman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I have one more tiny request, and if you want you can squash this into a single commit using the commit template or I can squash merge this myself when you're done.

@@ -18,8 +18,11 @@ export const handler = (argv: Arguments) => {
ensureConnection(argv, true);
};

let xud: any;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit, but this can be XudClient instead of any.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure thing

@rsercano
Copy link
Collaborator Author

rsercano commented Jan 15, 2019

If you may squash and merge I would be happier, don't want to take risk to break commit history of master :) @sangaman

@sangaman sangaman merged commit 6084043 into ExchangeUnion:master Jan 15, 2019
@ghost ghost removed the in progress label Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command line (CLI) Relating to the command line interface tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

streamOrders - wait for XUD
3 participants