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

connections should be transparent #8

Closed
timoxley opened this issue Jun 23, 2015 · 7 comments
Closed

connections should be transparent #8

timoxley opened this issue Jun 23, 2015 · 7 comments

Comments

@timoxley
Copy link

Piping stdout downstream.

Perhaps there's a good use-case for this but by default it would be nice if connecting from a remote did nothing but connect up the repl. If someone wants to suck the stdout into their connected client, then they could make a command for that (or you could make it a standard built-in command).

In addition, it probably shouldn't redirect output totally, rather it should just duplicate the output.

@dthree
Copy link
Owner

dthree commented Jun 24, 2015

Okay, so this is what I've been running into on this:

As Node is single-threaded, it's hard to duplicate transparent sessions (though I totally agree) in terms of sorting through stdout.

The only way I can think of doing this is to require logging on all vantage.command(...).action() actions to use Vantage's own logging method, i.e. this.log, or this.warn, or this.error. In this way, the logging can stay in that session's 'domain' and then I can accomplish perfect transparency (with an option for piping, as you went over).

I've gone back and forth on this several times in the past two weeks, as requiring users remember to use vantage.log for proper functionality seems like it has something wrong with it. I could be wrong.

Any input you have on the matter is appreciated.

@dthree
Copy link
Owner

dthree commented Jun 24, 2015

BTW, a REPL vantage extension is coming soon... with an additional bonus...

@timoxley
Copy link
Author

As Node is single-threaded, it's hard to duplicate transparent sessions (though I totally agree) in terms of sorting through stdout.

Oh, sorry, I don't follow, can you please elaborate?

@dthree
Copy link
Owner

dthree commented Jun 24, 2015

Sure.

Say your remote vantage server has this command:

vantage.command('foo')
  .action(function(args, cb){
    console.log('foo');
    cb();
  });

Now, suppose 3 clients log in to this server at the same time. Client 2 runs foo. Remote server throws foo to stdout.

We can do a few things here:

  1. Log foo on the remote tty (don't eat it),
  2. Pipe foo back to all 3 sessions logged in.

What we can't do is:

  1. Only pipe foo back to the session that originated it,
  2. Log other logging originated (disassociated with vantage) on the remote tty, while skipping the logging of foo on the remote tty.

In other words, the remote server has no way of knowing who originated what logging, as all stdout is on a single thread. When a vantage action runs, I know who is running the action, so if you used this.log (vantage being this), I could get smart and discern what logging originated from Vantage.

So there's a few ways to do this, but I'm not sure what the best way is.

Does that make any more sense? If I'm totally misduplicating your issue, just let me know.

@timoxley
Copy link
Author

makes sense, not sure the answer. I guess reason i opened this issue was that the current behaviour was surprising.

@dthree
Copy link
Owner

dthree commented Jul 16, 2015

Don't worry - it was a good point. Mid a large refactor (https://github.com/dthree/vantage/tree/authentication) which is doing a thorough fix on the whole subject you brought up. I'll let you know when it's good to go.

@dthree
Copy link
Owner

dthree commented Jul 25, 2015

Finished session refactor - all logging now goes through this.log (added details in docs), and the behavior is as you would expect.

Otherwise, many other updates were completed including authentication, and Vantage is production ready at 1.0! Hope you have a change to try it out.

@dthree dthree closed this as completed Jul 25, 2015
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

No branches or pull requests

2 participants