Odoo only depends on db if not using external db #159
Conversation
devel.yaml.jinja
Outdated
| @@ -48,7 +48,9 @@ services: | |||
| - ./odoo/auto:/opt/odoo/auto:rw,z | |||
| depends_on: | |||
| - cdnjs_cloudflare_proxy | |||
| {% if postgres_version -%} | |||
There was a problem hiding this comment.
Our code standard here for these kind of blocks is {%- ... %}.
Whitespace is weird in jinja and important in yaml.
Please follow it wherever possible (and here is possible). Same for all others.
There was a problem hiding this comment.
Ok. I actually did this following your suggestion in abad703
I guess it doesn't apply the same here, as we are modifying a yaml array, and not adding the service, as before, right?
There was a problem hiding this comment.
Shouldn't we put the same condition on 'pgweb' dependency and on the 'db' under volumes as well?
There was a problem hiding this comment.
I guess it doesn't apply the same here, as we are modifying a yaml array, and not adding the service, as before, right?
Well, your code is not wrong here, but it was wrong there. 😋
the - is telling jinja to remove all whitespace from (or to) that boundary. Wherever possible, the code standard here is to remove previous whitespace, and here it is possible. In the other code you link, it was not possible (it removed one extra line).
Shouldn't we put the same condition on 'pgweb' dependency and on the 'db' under volumes as well?
Yes, on every service that depends on db actually
Nope. Only on prod.yaml.
There was a problem hiding this comment.
In that case, IMOH below choice should be removed from copier question:
I will use an external PostgreSQL server: null
And also it is not clear in docs that how to change DB parameters when deploying in production. It will be good to add some help for that.
There was a problem hiding this comment.
Nope. Only on prod.yaml.
In that case, if the db service should always exist in devel and test, maybe we should end up with a different copier answer or mechanism to set up the case when we want an external DB on prod. If we set this var to null, we can't know which version to use in devel and test, right?
Maybe add another question with "Whether or not ti use external server?", that would override the db configuration only in prod.
There was a problem hiding this comment.
Maybe add another question with "Whether or not ti use external server?", that would override the db configuration only in prod.
Or maybe split db questions for prod and dev/test. For instance, there are separate questions for production (domains_prod) and test domains (domains_test).
There was a problem hiding this comment.
Yes, we have to rethink the UX with this question.
joao-p-marques
force-pushed
the
fix-db-depends
branch
from
b5f532f
to
a107a05
Compare
|
@yajo I'm trying to ressurect this old PR with a different approach to the UX, following the suggestions above. WDYT? |
…l db Fixes #150 (comment) The DB service is always created in common and imported in devel and test, but not in prod. The dependency between services also reflects that. By default, the devel and test environments will use the latest PG version.
joao-p-marques
force-pushed
the
fix-db-depends
branch
from
a107a05
to
fc34ad5
Compare
|
Possibly all will be easier with copier 6 and |
Odoo can't always depend on
dbbecause we don't always have it. This fixes that bug.Fixes #150 (comment)