PROPOSAL: .env secret and .env_share variables connected to odoo options should have the same name, perhaps capitalized

[gfcapalbo]

proposal:

it would be easier for all if the env variables described in .env_secret and .env_shared , that then correspond to odoo cfg options had the same name:

         for example variable $DEMO is then transformed in:  without_demo option, we should call it  $WITHOUT_DEMO
        https://github.com/sunflowerit/waftlib/blob/master/install#L21

other examples here: https://github.com/sunflowerit/waftlib/blob/master/templates/16.0/odoo.cfg#L8-L9

             $PGDATABASE becomes db_name,  $ADMIN_PASSWORD becomes admin_passwd,  $PGHOST becomes db_host

some correspond, others are a bit different. If would be cleaner if there was a 1 to 1 correspondence. If the change does not cost a lot of work , in terms of side effects on deployed systems, it would be a nice improvement.

[gfcapalbo]

In particular the $DEMO variable is a hurdle for newbies.

DEMO="true" means WITHOUT_DEMO = true , so it's semantically opposite.

[thomaspaulb]

@Hussam-Suleiman Currently, the build process makes a combination of everything that is in common/conf.d in alphabetical order, so if you have both odoo.cfg and override-odoo.cfg in that folder, it will combine the both of them and the override-odoo.cfg will take precedence because it has the highest place in the alphabet.

Maybe we could use that in our system too, so that instead of using variables, we can just override the values?

[Hussam-Suleiman]

@gfcapalbo other examples here: https://github.com/sunflowerit/waftlib/blob/master/templates/16.0/odoo.cfg#L8-L9 no. I don't agree with you, odoo variables are just inside odoo code, PGDATABASE is the default variable name for postgresql and we use it in the scripts and ansible, and the same variables names will make things easy for the programer byt hard to manage in the system, shell variables should not have the same name with code variables, at least if one small letters another should be capital

[Hussam-Suleiman]

@thomaspaulb sorry, I didn't understand what you meant, variables and value is a pair , if we use the value that means we use a variable. and override-odoo.cfg is a variables file store and it just overrides the variables inside odoo.cfg

what do you think?

[Hussam-Suleiman]

@gfcapalbo I have my reasons to make the variables with different names, then I can grep it and sed it by scripts to manage a big environment . like when I apply one thing to 150 containers, if I will change it manually I need one day to finish it, what is your reason to make it has same names?

[gfcapalbo]

@Hussam-Suleiman I see PGDATABASE , is used by sysadmin. so it cannot be removed, it is needed by system. Possible solution 1: Would supporting $PGDATABASE AND $DB_NAME , be viable? perhaps as so:

         db_name = $DB_NAME || $PGDATABASE     

there is no need to remove $PGDATABASE and the other PG variables, just make the ODOO variables availiable.

Possible solution 2: accept DB_NAME, and then MAP IT TO PGDATABSE. the same for all PG variables. if DB_NAME is set we then set the PG* variables too,

third point :
even if we are forced to leave PG* variables as they are, perhaps a small rework of other variables like DEMO could make waft more accessible for beginners.

I realize it's a big change, i just put out my opinion from a less experienced user. if we make the sourced variables adherent to odoo options, it will be easier for starters to adopt waft.

as for override-odoo.cfg @thomaspaulb i did not understand either.

[gfcapalbo]

In general, i just noted a small difficulty, if the solution is more confusing than the problem, close this issue. the final objective would be simplification, not confusion. We could solve this also by adding these details to documentation.

[gfcapalbo]

@gfcapalbo I have my reasons to make the variables with different names, then I can grep it and sed it by scripts to manage a big environment . like when I apply one thing to 150 containers, if I will change it manually I need one day to finish it, what is your reason to make it has same names?

Just developer comfort. but if it messes up sysadmin we leave everything as is The needed variables for sysadmin should remain. Consider the option of renaming DEMO and non-system (postgres) variables. I can make an MR to change $DEMO to $WITHOUT_DEMO

[thomaspaulb]

@gfcapalbo adding things in common/conf.d/override-odoo.cfg is just another way of modifying what variables are being used, I use that in some of the builds. common/conf.d/odoo.cfg is then a symlink to the Waft templates, and the other file is project-specific.

But the variables are another way of doing that.

I agree with Hussam's point that we cannot always map the variables 1:1 with the Odoo configuration names, because the variables can be used throughout the whole of Waft, in the build scripts, in other files, ... so I don't think that's a viable idea.

I'm good with changing some of the variables names, WITHOUT_DEMO is better than DEMO. But then again, simplification is better, so why do we use this variable at all? I would rather remove it completely. If we want to create a demo database, it can be done with a parameter to the install script.

[gfcapalbo]

@thomaspaulb question:

    by reading code i see  common/conf.d/override-odoo.cfg  override any settings coming from the .env-*   files
    correct?       

So we have 2 methods of customization:

    first  sourcing variables in the two  .env-* files, and using them in common/conf.d/odoo.cfg where available.
    then the override cfg, that could contain variables, or hardcoded values.

    correct?

Nevertheless will post the MR to change $DEMO name after workstuff done. looks like a nice clarification and does not bother sysadmin vars ( I think not, @Hussam-Suleiman can confirm?) Sorry for the noise. will post MR with some non-sysadmin var relabeling , not hard, then you consider if it merits a merge.

[thomaspaulb]

@gfcapalbo if common/conf.d/odoo.cfg contains a $VARIABLE name, and common/conf.d/override-odoo.cfg includes the same key but without that variable name, then the variable will indeed not be used. I agree that can be confusing, but I don't know a solution to it just now.

You don't agree to just remove DEMO altogether?

[Hussam-Suleiman]

@thomaspaulb @gfcapalbo I found DEMO variable just in https://github.com/sunflowerit/waftlib/blob/master/install#L18 and I changed it, and I added a few comments to explain where we use some variables.

[thomaspaulb]

Yes but in that place it's not really using the DEMO variable, it's just using a command line parameter.

[Hussam-Suleiman]

true, just I changed it because I found changing it will be very easy 😂