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

docs: Managing microservices dependencies first draft #25

Merged
merged 18 commits into from
Jun 19, 2024
Merged

Conversation

john-gom
Copy link
Collaborator

What

ADR for service dependencies

TBA: We should proably create a team for people who should review these and add that to the CONTRIBUTORS file

Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

So far, good to me :-)

docs/decisions/managing-service-dependencies.md Outdated Show resolved Hide resolved
@teolemon teolemon changed the title Managing microservices dependencies first draft docs: Managing microservices dependencies first draft Apr 26, 2024
Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
@john-gom
Copy link
Collaborator Author

Hi @raphael0202, @alexgarel , @stephanegigandet, @teolemon I've added a ports section which is something we discussed a while back. It would be good to go back to existing projects and adjust the port numbers to avoid clashes and add them to this list

Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
@alexgarel
Copy link
Member

@john-gom there are services where we don't necessarily need other projects running to develop. This is the case for example for robotof, where we just need to get data for the mongoDB, same for search-a-licious you just need to import some ES data. But you might need it all to test integration.
Should we also formalize the way we do it?

@teolemon
Copy link
Member

@alexgarel (typed with Neuralink™)

@john-gom
Copy link
Collaborator Author

Hi @alexgarel , yes there are different levels of dependency. If we have hard dependencies like mongodb and redis then maybe these should be in their own separate repos? Certainly something to discuss some more...

Signed-off-by: John Gomersall <thegoms@gmail.com>
@hangy
Copy link
Member

hangy commented May 6, 2024

One issue that I noticed when testing the PoC in the keycloak branch: If a project (in this case openfodfacts-auth) has a dependency that the calling project (here openfoodfacts-server) doesn't require (here: node.js to run node build-scripts/build_languages.mjs), then this implicitly becomes a direct dependency of the project where make run is called from. I have a fear that this won't work well in more complex scenarios.

I don't know if this works with all projects, but one option might be to require using docker run or docker compose run, like we do when npm needs to be used in ProductOpener when running targets like front_build. Otherwise, I fear that the plethora of dependencies that one needs to install locally will be way too difficult for people to easily jump into development.

@john-gom
Copy link
Collaborator Author

john-gom commented May 6, 2024

One issue that I noticed when testing the PoC in the keycloak branch: If a project (in this case openfodfacts-auth) has a dependency that the calling project (here openfoodfacts-server) doesn't require (here: node.js to run node build-scripts/build_languages.mjs), then this implicitly becomes a direct dependency of the project where make run is called from. I have a fear that this won't work well in more complex scenarios.

Hi @hangy , there shouldn't be a need to run any scripts locally when dependencies are pulled in, I'll have to investigate what's going on if that is the case with openfoodfacts-auth

@hangy
Copy link
Member

hangy commented May 6, 2024

there shouldn't be a need to run any scripts locally when dependencies are pulled in, I'll have to investigate what's going on if that is the case with openfoodfacts-auth

Don't worry. Maybe I mixed something up or made a mistake. Will have another look tomorrow!

Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
@john-gom
Copy link
Collaborator Author

john-gom commented May 13, 2024

Hi all, I've had an idea regarding not loading dependencies into the parent folder. We could introduce a "DEPS_DIR" environment variable and if this is not set then dependencies will load into a "deps" directory of the current project, thus avoiding going to the parent of the current directory. If someone wants to develop on multiple projects simultaneously then they could set the DEPS_DIR to the parent of their working directories. Code might be something like this:

load_dependencies:
	@if [ -z "$$DEPS_DIR" ]; then export DEPS_DIR=$$PWD/deps; fi; \
	mkdir -p $$DEPS_DIR; \
	for dep in "project1" "project2" ; do \
		if [ ! -d $$DEPS_DIR/$$dep ]; then \
			git clone --filter=blob:none --sparse \
				https://github.com/openfoodfacts/$$dep.git $$DEPS_DIR/$$dep; \
		else \
			cd $$DEPS_DIR/$$dep && git pull; \
		fi; \
		cd $$DEPS_DIR/$$dep && make -e run; \
	done

If there is recursion then the recursed project will still be cloned into the deps folder but it won't continue recursing because the "{PROJECT1}_RUNNING" variable will be set.

It does make it a little bit more complicated, but at least people who are just working on one repo won't see side effects in the parent of their working folder and might avoid issues with online development environments like codepages.

What do other think? cc @hangy @alexgarel @stephanegigandet @raphael0202 @teolemon

@alexgarel
Copy link
Member

We could introduce a "DEPS_DIR" environment variable

Good idea @john-gom

If someone wants more granularity, he can even use symlinks in the deps/ folder to reuse an existing directory.

Signed-off-by: John Gomersall <thegoms@gmail.com>
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

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

Great, thanks !

Just two small remarks.

docs/decisions/managing-service-dependencies.md Outdated Show resolved Hide resolved
docs/decisions/managing-service-dependencies.md Outdated Show resolved Hide resolved
Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
Signed-off-by: John Gomersall <thegoms@gmail.com>
Comment on lines +60 to +61
# Use override here to ensure this is not inherited from the environment
override DOCKER_COMPOSE=COMPOSE_PROJECT_NAME=${PROJECT1_PROJECT_NAME} COMPOSE_FILE="${PROJECT1_COMPOSE_FILE}" docker compose
Copy link
Member

Choose a reason for hiding this comment

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

This will also affect ordinary commands, won't it ?

Copy link
Member

Choose a reason for hiding this comment

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

If I understand it well, in off-server we have to define a OFF_SERVER_PROJECT_NAME ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not yet. It is only a problem when you are working on a project that needs to be pulled in as a dependency as you don't want any environment variables from the depending (parent) project to overwrite your own.

Copy link
Member

Choose a reason for hiding this comment

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

Cool !

Copy link
Member

Choose a reason for hiding this comment

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

Do you have the source for the chart. Can you put it there, or a reference URL ?

Copy link
Member

Choose a reason for hiding this comment

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

@john-gom john-gom merged commit 8717b56 into main Jun 19, 2024
1 check passed
@john-gom john-gom deleted the microservices branch June 19, 2024 10:31
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.

None yet

5 participants