Skip to content

Allow streaming when using exec#272

Open
Intrepidd wants to merge 2 commits into
upserve:masterfrom
Intrepidd:patch-1
Open

Allow streaming when using exec#272
Intrepidd wants to merge 2 commits into
upserve:masterfrom
Intrepidd:patch-1

Conversation

@Intrepidd

Copy link
Copy Markdown

Related to #263

@Intrepidd

Copy link
Copy Markdown
Author

Cassettes need to be re-recorded, do you want me to do it ?

@bfulton

bfulton commented Apr 4, 2015

Copy link
Copy Markdown
Contributor

Thanks! Yes, please re-record.

@Intrepidd

Copy link
Copy Markdown
Author

Running rake vcr:spec on master throws some errors for me :

rspec ./spec/docker/container_spec.rb:113 # Docker::Container#rename renames the container
rspec ./spec/docker/image_spec.rb:314 # Docker::Image.create when the Image does not yet exist and the body is a Hash sets the id and sends Docker.creds
rspec ./spec/docker/image_spec.rb:375 # Docker::Image.save when a filename is specified exports tarball of image to specified file
rspec ./spec/docker/image_spec.rb:383 # Docker::Image.save when no filename is specified returns raw binary data as string
rspec ./spec/docker_spec.rb:181 # Docker#info returns the info as a Hash

I'm on OSX Yosemite with the latest boot2docker.

I've done everything stated in TESTING.md

Not sure if the culprit is boot2docker or anything else going wrong on my system.

@Intrepidd

Copy link
Copy Markdown
Author

I only re-recorded the cassettes I needed and now it's green :)

@bfulton

bfulton commented Apr 6, 2015

Copy link
Copy Markdown
Contributor

Hmm. CI is still failing. I can take a look this evening.

@Intrepidd

Copy link
Copy Markdown
Author

It's because of a complexity violation :/

On Mon, Apr 6, 2015, 15:48 Bright Fulton notifications@github.com wrote:

Hmm. CI is still failing. I can take a look this evening.


Reply to this email directly or view it on GitHub
#272 (comment).

@tlunter

tlunter commented Apr 7, 2015

Copy link
Copy Markdown
Contributor

Can you share how you're going about the streaming?

I used the following with a simple container running just bash and it's streaming for me just fine.

c = Docker::Container.all.first
c.exec(["/bin/bash","-c","while [ 1 ]; do echo hello; sleep 5; done"]) do |stream, chunk|
  puts "#{stream}: #{chunk}"
end

@Intrepidd

Copy link
Copy Markdown
Author

I'm doing exactly the same way, and I only get a response when the command
is finished.

I'm on OSX and using boot2docker, not sure if this changes anything

On Tue, Apr 7, 2015, 21:49 Todd Lunter notifications@github.com wrote:

Can you share how you're going about the streaming?

I used the following with a simple container running just bash and it's
streaming for me just fine.

c = Docker::Container.all.first
c.exec(["/bin/bash","-c","while [ 1 ]; do echo hello; sleep 5; done"]) do |stream, chunk|
puts "#{stream}: #{chunk}"
end


Reply to this email directly or view it on GitHub
#272 (comment).

@Intrepidd

Copy link
Copy Markdown
Author

I ran your code to be extra sure and I'm having no output at all since it's an infinite loop.

This is very strange that you're getting it to work on your development environment

@tlunter

tlunter commented Apr 7, 2015

Copy link
Copy Markdown
Contributor

I wonder if this has to do with SSL. You're using one of the latest boot2docker, right?

@Intrepidd

Copy link
Copy Markdown
Author

The latest, and I have to disable ssl verification

On Tue, Apr 7, 2015, 23:04 Todd Lunter notifications@github.com wrote:

I wonder if this has to do with SSL. You're using one of the latest
boot2docker, right?


Reply to this email directly or view it on GitHub
#272 (comment).

@tlunter

tlunter commented Apr 8, 2015

Copy link
Copy Markdown
Contributor

I have done the same and using the code above, this PR isn't working for me. I am thinking this has to do with Excon's SSLSocket versus regular Socket. Using tcpdump I can see the 5 second echo payload, but cannot see Excon producing anything from it. Will need to look into deeper.

@danielmanesku

Copy link
Copy Markdown

@tlunter any updates on this one?
I tried to apply those changes, and it does not seem to work for me.

@tlunter

tlunter commented Jun 27, 2016

Copy link
Copy Markdown
Contributor

@danielmanesku I haven't been able to look into this one since my last post. Would love it if you could take a deep dive if you're available to do so.

@danielmanesku

Copy link
Copy Markdown

Thanks for a quick reply! I am busy at the moment with something else, but I added this on my TODO list and I will try to investigate in the coming weeks.

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.

4 participants