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

Fixing docker internal hosts (option 2) #1987

Closed
wants to merge 1 commit into from

Conversation

amazingphilippe
Copy link
Contributor

@amazingphilippe amazingphilippe commented Nov 1, 2024

Summary | Résumé

This will need a proper review. While it works, I'm not sure why.

Essentially, this PR should use docker-compose.yml to make sure we use a new "notification-network" in both api and admin ymls. See related PR here: cds-snc/notification-api#2340

Did I configure Dockerfile, devcontainer.json properly? I doubt it.

Additionally, when opening this container, Poetry complained about not being able to install "notification-admin". Possibly because packages is empty in pyproject.toml? Again, please code check me!

Test instructions | Instructions pour tester la modification

Sequential steps (1., 2., 3., ...) that describe how to test this change. This
will help a developer test things out without too much detective work. Also,
include any environmental setup steps that aren't in the normal README steps
and/or any time-based elements that this requires.


Étapes consécutives (1., 2., 3., …) qui décrivent la façon de tester la
modification. Elles aideront les développeurs à faire des tests sans avoir à
jouer au détective. Veuillez aussi inclure toutes les étapes de configuration
de l’environnement qui ne font pas partie des étapes normales dans le fichier
README et tout élément temporel requis.

Copy link

github-actions bot commented Nov 1, 2024

@amazingphilippe
Copy link
Contributor Author

Closing this for #1986

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.

1 participant