docker-sync icon indicating copy to clipboard operation
docker-sync copied to clipboard

Remove dependency on docker-compose gem; support Ruby 3.x

Open takahashim opened this issue 2 years ago • 7 comments

Hi, I'm new to this gem, but my team is using docker-sync for daily development. Thank you very much for this useful gem!

To fix #777, I tried to do #788, took only the minimal functionality from docker-compose and backticks gems and combined it into the DockerSync::DockerComposeSession and DockerSync::Command classes.

These classes are limited in functionality and less flexible (ex. arguments of Command.run is just Array of String, not Hash). However, it is rather desirable in terms of stability, and we can add more functions if needed (to solve #787 or other issues).

takahashim avatar Feb 20 '22 05:02 takahashim

This sounds great already! I guess we have to be somewhat careful to understand how the current options could parametrise docker-compose so we can keep the same level at least

The reason to get rid of the gem was

A) more flexibility and native/direct options support B) docker-compose configuration file overloading C) prepare ruby 3 support

I think the most sensible/ wanted part was b)

Did you already scan the configuration for options we need to pipe?

Thank you so much for working on this!

EugenMayer avatar Feb 22 '22 06:02 EugenMayer

Thank you for your reply! I'm glad you like it.

The docker-compose CLI options that this PR supports may still be insufficient, but we currently support the following options:

docker-compose up

  • docker-compose up (uses default docker-compose.yml)
  • docker-compose --file=foo.yml up
  • docker-compose --file=foo.yml --file=bar.yml (... or more --file options) up
  • docker-compose --file=foo.yml --file=bar.yml up --build

docker-compose stop

  • docker-compose stop
  • docker-compose --file=foo.yml --file=bar.yml (...) stop

docker-compose down

  • docker-compose down
  • docker-compose --file=foo.yml --file=bar.yml (...) down

I've modified the spec and added a few more. I think this makes it easier to understand what the DockerComposeSession class currently supports.

https://github.com/EugenMayer/docker-sync/blob/2b19fe696afb7a7aa311c8d412f3b24d04e3e547/spec/lib/docker-sync/docker_compose_session_spec.rb#L22-L93

A) more flexibility and native/direct options support

For this point, what DockerComposeSession actually doing is just pass command arguments as an Array of String to the Kernel#spawn method; so it's direct enough, I guess.

B) docker-compose configuration file overloading

As I wrote above, the (multiple) --file options of docker-compose CLI is supported.

takahashim avatar Feb 23 '22 07:02 takahashim

By the way, I'm using --file=foo.yml style for file options to match the current behavior (I think), but should I use -f foo.yml style with short options? (Because the manual of docker-compose was the latter.)

takahashim avatar Feb 23 '22 07:02 takahashim

Really nice.

A really nice take-away from this PR would be the support of .env and COMPOSE_FILE in there, so one can have customer overrides per .env for docker-compose. This was relevant for macos and linux teams, where the latter wanted to use docker-sync, but basically as a dummy - and using an .env file for that makes a lot of sense.

I will review the PR ASAP so we can move on with the issue. I think you are basically tackling several issues and features with this single PR at the same time.

Thank you for your work!

EugenMayer avatar Feb 23 '22 08:02 EugenMayer

@takahashim sorry for the huge delay on this one. I would like to pick this one up since i think your work is really valueable and a lot of effort has been put into that already.

I'am sure you are running your variant for a while now, anything suprising we need to consider? Are you still using the spin off in production?

EugenMayer avatar May 06 '22 06:05 EugenMayer

@takahashim is there any chance to get you motivated or any type of help i could provide so you finish this really valuable effort? Let me know!

EugenMayer avatar May 16 '22 06:05 EugenMayer

@takahashim i re-rolled your branch and merged downstream in #803 since - let me know if you want to continue here, happy to close the other branch in favour of your work here. This or that way, all the credits for the work are yours.

EugenMayer avatar May 22 '22 19:05 EugenMayer