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

commands output unable to stream #453

Closed
whyrusleeping opened this issue Dec 15, 2014 · 13 comments
Closed

commands output unable to stream #453

whyrusleeping opened this issue Dec 15, 2014 · 13 comments

Comments

@whyrusleeping
Copy link
Member

It seems to me that all the output for a given command has to be printed at once. Im working on implementing ipfs ping and this model doesnt really work, as ping needs to print out a message after each successful (or failed) ping operation. Am I missing a way to do this? or is that not possible?

cc @mappum

@btc
Copy link
Contributor

btc commented Dec 15, 2014

The DAG Reader (ipfs cat) is one place where we stream output. That pattern may work here.

https://github.com/jbenet/go-ipfs/blob/master/core/commands/cat.go#L37

@mappum
Copy link
Contributor

mappum commented Dec 16, 2014

The DAG Reader (ipfs cat) is one place where we stream output. That pattern may work here.

https://github.com/jbenet/go-ipfs/blob/master/core/commands/cat.go#L37

That does work, but would lose the functionality from the encoding system. (You wouldn't be able to get machine-readable output unless you reimplement the handling for the --encoding flag).

I could modify the commands library to let commands return channels, then the encoding can work like it does now by separately encoding each object received from the channel. (Commands that return channels would do their business logic in a goroutine). Consumers could still get the output as one big array if they want, so this won't break anything.

Thoughts?

@whyrusleeping
Copy link
Member Author

returning channels would be a pretty interesting way to retain the machine readability of the output. @jbenet @maybebtc what do you guys think?

@btc
Copy link
Contributor

btc commented Dec 16, 2014

I can't readily visualize how this would work in practice and mix with the rest of the system, so I'm afraid I cannot be of much assistance.

@jbenet
Copy link
Member

jbenet commented Dec 16, 2014

not channels, use io.Reader/Writers. so we can io.Copy(out, bigfile)

@whyrusleeping
Copy link
Member Author

channels make more sense for ping though

@jbenet
Copy link
Member

jbenet commented Dec 16, 2014

not necessarily. it's outputting io to the console.

@whyrusleeping
Copy link
Member Author

mappums argument for the channels was so that we could still retain machine readability using objects and marshallers, we would lose that with a reader/writer

@mappum
Copy link
Contributor

mappum commented Dec 16, 2014

not channels, use io.Reader/Writers. so we can io.Copy(out, bigfile)

We actually already support io.Readers (that's how ipfs cat works), but that won't get to use any of the code already in plane for handling encodings.

I already did some of the work yesterday making command marshalers return io.Readers instead of []bytes, but I found that the golang stdlib http.ResponseWriter doesn't let us do our own chunking for the output, so we can't stream (it does automatically for large files though, which is why it works with cat). The solution may be to make a non-HTTP command transport, but that will be a lot of work.

Might be best to punt and do it with an io.Reader for now, but ignore encoding (or handle it inside the command).

@jbenet
Copy link
Member

jbenet commented Dec 18, 2014

http.ResponseWriter doesn't let us do our own chunking for the output

Why not http://golang.org/pkg/net/http/#Flusher ?

The solution may be to make a non-HTTP command transport, but that will be a lot of work.

Agreed, let's not do that.

but ignore encoding (or handle it inside the command).

Yeah... it's probably for the best. what we could do is wrap the ResponseWriter with our own io.Writer style thing that uses interfaces and not directly bytes:

type CodecWriter interface {
  Write(t interface{}) error
}

type CodecReader interface {
  Read(t interface{}) error
}

type jsonWriter struct {...}

func (jw *jsonWriter) Write(t interface{}) error {
  enc, err := json.Marshal(t)
  if err != nil {
    return err
  } 

  for t := 0; t < enc; {
    n, err := jw.w.Write(enc)
    if err != nil {
      return err
    }
    t = t + n
  }
  return nil
}

@jbenet
Copy link
Member

jbenet commented Dec 18, 2014

Actually im sure this exists already. ... and yep:

probably need to wrap these in a common interface (they should all be Encoder and Decoder probably)

@jbenet
Copy link
Member

jbenet commented Dec 18, 2014

More on Flusher:

func handle(res http.ResponseWriter, req *http.Request) {
  fmt.Fprintf(res, "sending first line of data")
  if f, ok := res.(http.Flusher); ok {
     f.Flush()
  } else {
     log.Println("Damn, no flush");
  }
  sleep(10) //not real code
  fmt.Fprintf(res, "sending second line of data")
}

@whyrusleeping
Copy link
Member Author

I think I can mark this closed for now, @mappum implemented channels for streaming output back.

ariescodescream pushed a commit to ariescodescream/go-ipfs that referenced this issue Oct 23, 2021
Related to ipfs#453 but not a fix. This will cause us to actually return early when
we start blocking on sending to some peers, but it won't really _unblock_ those
peers. For that, we need to write with a context.
@aschmahmann aschmahmann mentioned this issue Dec 1, 2021
80 tasks
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

4 participants