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

Use a custom docker-file.override.yml #158

Merged
merged 1 commit into from
Jul 31, 2019
Merged

Use a custom docker-file.override.yml #158

merged 1 commit into from
Jul 31, 2019

Conversation

dpordomingo
Copy link
Contributor

@dpordomingo dpordomingo commented Jul 21, 2019

blocks src-d/sourced-ui#221
required by #26

In order to use the default feature of Docker Compose to automatically load an extra docker-compose.override.yml file (docs multiple-compose-files), if this PR is accepted, it will be created a default (empty) docker-compose.override.yml, that will be linked in the workdirs (as done with the active docker-compose.yml)

The default docker-compose.override.yml is generated using the same version than the active docker-compose.yml

Usage:

  1. User inits
  2. then ~/.sourced/compose-files/__active__/docker-compose.override.yml is modified
  3. apply changes with sourced restart

I think we can consider this as an internal feature (not documented), used for people willing to manually modify the ~/.sourced/compose-files/__active__/docker-compose.yml, that is also not documented.

@dpordomingo dpordomingo added the enhancement New feature or request label Jul 21, 2019
@dpordomingo dpordomingo requested a review from a team July 21, 2019 16:42
@dpordomingo dpordomingo self-assigned this Jul 21, 2019
@dpordomingo dpordomingo changed the title Let sourced to recognize a custom docker-file.override.yml Use a custom docker-file.override.yml Jul 21, 2019
@se7entyse7en
Copy link
Contributor

Isn't it easier to always have a docker-compose.override.yml which by default it empty and to always have it linked instead of adding/removing? Does it work having an empty docker-compose.override.yml file?

@smacker
Copy link
Contributor

smacker commented Jul 24, 2019

Agree with @se7entyse7en let's test if empty docker-compose.override.yml works

@dpordomingo
Copy link
Contributor Author

tested.
Does not work with an empty file, but it does if docker-compose.override.yml contains only

version: '3.2'

Because it's a valid docker-compose.yml file.

Will it satisfy your suggestion @smacker @se7entyse7en ?

@se7entyse7en
Copy link
Contributor

Sure, my definition of empty was actually empty-ish. 😛

@smacker
Copy link
Contributor

smacker commented Jul 24, 2019

Good. If it makes the code simpler let's use it.

@dpordomingo
Copy link
Contributor Author

Done @se7entyse7en @smacker

  • 7c720d0 is not needed for this functionality, but a cleanup useful to avoid polishing workdirs
  • 323a0c0 implements what you requested

@dpordomingo dpordomingo requested a review from smacker July 25, 2019 15:50
@dpordomingo
Copy link
Contributor Author

Thanks for your deep review @smacker
I think I addressed all your suggestions.

Copy link
Contributor

@smacker smacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. Much simpler.

@dpordomingo dpordomingo requested a review from se7entyse7en July 26, 2019 04:17
@se7entyse7en
Copy link
Contributor

Sorry for the delay, I'm gonna have another look 👍

@dpordomingo dpordomingo requested a review from se7entyse7en July 26, 2019 12:16
@dpordomingo
Copy link
Contributor Author

Great!
Now we have an agreement, this is ready for the last review from @carlosms (who he is coming back "tomorrow" Monday)

@carlosms
Copy link
Contributor

Code looks ok.

Before merging I'd like to ask if there are any other alternatives that you considered.
I mean, instead of supporting this .override file in sourced-ce, could the makefile in sourced-ui place the override file directly in ~/.sourced/workdirs/__active__/?
Or somehow use the docker-compose --file option (COMPOSE_FILE) to point to the normal docker-compose.yml plus the .override file in the sourced-ui repo?

@dpordomingo
Copy link
Contributor Author

I discarded both because I felt the developer experience using the "main override" will be more natural, and it could avoid extra steps when changing between workdirs and such. Once the development mode is enabled, filling the .override with some content, no matter how you run sourced, that it will use the same config.

For sure, the same thing can be achieved in some different ways, but what I thought about both alternatives is explained below

--file

it would require modifying sourced command to accept an extra argument or env value to define the path of the extra docker-compose config file.

  • I discarded this one because using an .override config is kind of the default way in docker-compose to override an original config. Also, since this method would not work at workdir level, there could be inconsistencies between terminal tabs.

override in workdirs/__active__

  • I discarded this way because it would require adding and deleting the .override config every time it's tested a new working directory.

When initializing a workdir, it will be linked the default
docker-compose.override.yml file, that could be used to override
sourced config.

If the default docker-compose.override.yml is deleted, it will
be restored everytime an init command is run, same as it is
currently done with the main docker-compose.yml.

Signed-off-by: David Pordomingo <[email protected]>
@dpordomingo dpordomingo merged commit 49636af into src-d:master Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants