Skip to content

Move web service to profiles for explicit startup #1337

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

Closed
wants to merge 1 commit into from

Conversation

richardhallett
Copy link
Contributor

This is a different way of specifying what comes up with docker compose

This is a breaking workflow change as by default with "docker compose up" it will not start the rails app "web"

Purpose

This is to allow for example local running of the rails app but still starting anxillery services needed by the app i.e. mysql, opensearch etc.
This is the same as using the docker compose file .local

If this PR is accepted seperately we might wish to remove the .local one.

Approach

Uses docker profiles https://docs.docker.com/compose/how-tos/profiles/

Open Questions and Pre-Merge TODOs

N/A

Learning

See approach

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

@richardhallett richardhallett requested review from ashwinisukale and a team May 6, 2025 16:07
@richardhallett richardhallett self-assigned this May 6, 2025
Copy link
Contributor

@ashwinisukale ashwinisukale left a comment

Choose a reason for hiding this comment

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

Thanks for this new way

@digitaldogsbody
Copy link
Member

Because I'm an awkward sod, I'm curious if this can be inverted? I.e the default is to start all services and the profile flag is used to prevent the rails container.

This feels like a more intuitive approach to me, in terms of keeping what is (I assume) the more common usage as the "base" operation.

I haven't looked at the docs for how the compose profiles work, so this idea may not actually be practical.

@digitaldogsbody
Copy link
Member

I just know I'm going to forget about this by the time I next spin up the lupo stuck locally and get very confused 😅

@richardhallett
Copy link
Contributor Author

I appreciate the concern, unfortunatly it's not too easy to do the opposite, because you can't specify profiles to disable.
There is a possible trick that you can specify the default profile using an empty string like "" and if you did it like that, then I could say the default is everything and still specify a profiles for just say "services".
This however might not work on all docker compose versions, it's not documented officially as far as I can tell, just some people reporting this as a possibility.

So I think maybe I won't merge this PR. I'll think about it some more and see if there is an alternative compromise.

The main goal of this was an example to show not requiring special docker compose files, but that perhaps is just the easiest solution.

@richardhallett
Copy link
Contributor Author

As discussed, deciding against for now as it breaks workflows for people.

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.

3 participants