dokku / dokku-daemon

A daemon wrapper around dokku
MIT License
32 stars 13 forks source link

Updates dokku-daemon for compatibility #25

Closed dean1012 closed 5 years ago

dean1012 commented 5 years ago
josegonzalez commented 5 years ago

I'll merge once tests pass.

josegonzalez commented 5 years ago

Just be sure to comment here when they do to remind me :)

dean1012 commented 5 years ago

@josegonzalez I am running low on time at the moment and don't have time to look at this right now. I'll get to it when I can but hopefully, someone else in the community will find time before I do.

I can confirm that this works for me when using the following:

dean1012 commented 5 years ago

@josegonzalez Just wanted to update you...

I've got all the testing fixed except one thing. The client_command function uses nc to pipe commands into the socket and get the result.

Unfortunately, it seems that nc is not sending the resulting output anywhere that I can find which causes all the tests relying on that function to fail.

I strongly believe that once this function is fixed, all tests will work.

I am continually looking into this but would appreciate any insight from the community as well.

Look forward to merging this.

Also, an unrelated tidbit... I ran into many problems with the dokku-api software not properly handling JSON returned by dokku-daemon for the logs command. I ended up making my own software based on dokku-api with a much simpler feature set and based on NodeJS. It is hosted at https://github.com/dean1012/dokku-api-nodejs

dean1012 commented 5 years ago

@josegonzalez Would you look at that? All tests pass!

To recap, this pull request essentially does the following:

Please merge when able! I am happy I was able to help this project out.

dean1012 commented 5 years ago

Added a last minute fix to a corner case bug. Should be good now once the tests pass.

dean1012 commented 5 years ago

Give me a little bit of time to make a new commit with requested changes and do some testing.

dean1012 commented 5 years ago

All passing and I believe I've addressed all your concerns now @josegonzalez - you can merge now.

dean1012 commented 5 years ago

@josegonzalez is there anything else you need me to do here before you can merge? Just checking, thanks!

dean1012 commented 5 years ago

Can we please merge this request @josegonzalez ? Thank you!