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

Allow selection of BE through env var #115

Closed
wants to merge 5 commits into from
Closed

Allow selection of BE through env var #115

wants to merge 5 commits into from

Conversation

minottic
Copy link
Contributor

@minottic minottic commented Apr 5, 2024

I started to have a look at this #107 and I think the PR here could work well.

It is split into commits to ease the review, I will open dedicated PRs if this looks ok.

The idea is to enable only one backend at a time, which can be configured through an ENV var. Then the other containers will connect to it without noticing the change (also the code does not require much change).

This relies on an include rule which depends on the ENV provided and on the fact that both be_next and be expose a container named backend, which the others connect to. This might also simplify in the future the connection to the archivemock

I still have to update the README (both the global and the backend one) and the CI

The drawback of this is that it does not support running both BEs at the same time, but this can be circumvented (I think) in a later PR by making the traefik port custom and setting the docker compose project-name. In this way, one can run:

docker-compose up -d 

which will run the services with the old BE

and then run, e.g.

export BE_VERSION=backendnext
export TRAEFIK_PORT=81
docker-compose -p $BE_VERSION up -d 

which will run the services with the new BE and expose them on port 81, ultimately creating 2 envs with all the services, one relying on the new BE and on port 81, another relying on the old BE and on port 80.

@minottic minottic requested a review from a team April 5, 2024 16:03
@minottic minottic marked this pull request as draft April 5, 2024 16:28
@fpotier
Copy link
Member

fpotier commented Apr 8, 2024

Just wondering how does it compare to the solution with profiles? I liked the idea of using them but I'm unsure they offer the same features

@consolethinks
Copy link
Contributor

Is there a need for both BE's to run at the same time?

@fpotier
Copy link
Member

fpotier commented Apr 8, 2024

I don't think so

@minottic
Copy link
Contributor Author

minottic commented Apr 8, 2024

I think the differences are:

  1. we don't need to duplicate all services for the 2 bes
  2. we don't need to tell the user what subdomain to use in each case
  3. we avoid the --profile * problem which would spin un both bes at the same time, making it confusing

I don't think we need both bes at the same time, but rather the 2 envs potentially, which could be covered by my example above

I think we will use profiles in the future, probably to bundle envs together, e.g. the one with the zip-service, the one without, but I think here we are in a slightly different use cases as we want to have different configurations (profiles) basing them on different BEs, so choosing the profile and the be it uses should be set by two different flags.

So, in general, I reckon useful to be able to select the BE, only allow one, and call the service backend, no matter the flavour (so that other services don't really care what flavour it is). Schematically:

  1. I think we should call both new BE and old BE service backend
  2. we can either make the user decide which through profile (if it works) or through the env. (the other advantage of the env var is that it has defaults, so in most cases the users will have to do nothing.

@minottic
Copy link
Contributor Author

minottic commented Apr 8, 2024

it will be closed by relative atomic PRs

@minottic
Copy link
Contributor Author

closed by related PRs

@minottic minottic closed this Apr 10, 2024
@minottic minottic deleted the select_be branch April 10, 2024 08:34
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