-
Notifications
You must be signed in to change notification settings - Fork 31
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
Upd use docker compose v2 fixes #159
Conversation
docky/cmd/run_open.py
Outdated
if self.root: | ||
user = "root" | ||
return user | ||
return self.project.get_user(service) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks root right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aahahha my finger didn't do what my brand was thinking !!
It's ok now
docky/cmd/run_open.py
Outdated
@@ -53,7 +52,7 @@ class DockyRun(DockyExec): | |||
"""Start services and enter in your dev container""" | |||
|
|||
def _check_running(self): | |||
if self.project.get_containers(service=self.service): | |||
if docker.compose.ps(services=[self.service], all=True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why all ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you do not pass all, you do not see the odoo container as it launch with "run". All is to show all container even if there are not launch with the normal way "docker compose up"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but will you get exited containers ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: in the previous version we had the same behavior
And normally you should never have exited container for the main container (as we launch it with --rm).
But if you have used manually docker compose run
and you didn't have launch docker compose rm
then you have an exited container and docky run
is blocked !
So we can first remove this existed container (as it will stay here indefinitely) and then start the container correctly. See commit : 0c60eeb
docky/common/project.py
Outdated
for _name, service in self.project.get("services").items(): | ||
docky_help = service.get("labels", {}).get("docky.help") | ||
if docky_help: | ||
logger.info(docky_help) | ||
|
||
def create_volume(self): | ||
"""Mkdir volumes if they don't exist yet. | ||
|
||
Only apply to external volumes. | ||
docker-compose up do not attemps to create it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docker-compose up do not attemps to create it | |
docker compose up create volumes automatically, but docker compose run don't |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in fact the comment was totally wrong docker compose always create the volume but with the user root. I fix it
f219a8c
to
cfb645a
Compare
@paradoxxxzero @hparfr updated |
Thank you @sebastienbeau , if you think will be better has a "clean" commits I can see to include your changes of:
During this time I'm also think in a way to make Docky more abstract, what a thought was allow change the default command $ docky run by a parameter in the docker-compose.yml file, for example if include something like: odoo: or in the .env odoo: This command will overridden the Default, with this we can allow each Project has, if necessary, a specific parameters Saved in the file and in the big projects the most part of the Developers can ignore those specific parameters by just typing docky run or the Docky can be use for projects beyond Odoo case, but it's just a test, a speculation, let me know if you think it can be useful or not. |
@mbcosta thanks for your feedback, I will squash the commit before merge them. Regarding the option of
|
I have added a task for the option on docky run /exec #160 |
- included library python-on-whales to get some Config information from compose.yml files. - Remove the logic of getting ip information as now we always use traefik - Raise the right error message when the config is not valid Co-authored-by: Sébastien BEAU <sebastien.beau@akretion.com>
…cker info' for check errors related to OS or older or newer libraries versions.
Remove dcpatched and hack for kill as docker compose work correctly Co-authored-by: Sébastien BEAU <sebastien.beau@akretion.com>
0c60eeb
to
9ced2c2
Compare
@mbcosta I have done some additionnal fixes on your PR
We can merge it and then release a new version.
The next step is to add some test (I will try to do it in a separated PR)