[5.0] price_accuracy in config file never use in create database

Bug #392114 reported by Christophe CHAUVET
22
This bug affects 4 people
Affects Status Importance Assigned to Milestone
Odoo Server (MOVED TO GITHUB)
Status tracked in Trunk
5.0
Fix Released
Medium
Christophe Simonis (OpenERP)
Trunk
Invalid
Undecided
Unassigned

Bug Description

Server revno: 1799
Addons revno: 2334
Client revno: 950

In config file, add:
price_accuracy = 5

launch the server with ./openerp-server.py -c ./file.conf

create a new database

in product_template, standard_price and list_price are numeric(16,2)

or launch the server with ./openerp-server.py -c ./file.conf --price_accuracy=5

create a new database

in product_template, standard_price and list_price are numeric(16,5)

If argument --price_accuracy is not specify, don't take 2 digits by default, use value in config file.

see patch to correct this issue

Distribution: Ubuntu
Version: 8.10 (intrepid)
Python 2.5.2

Locale:
  LANG=fr_FR.UTF-8

Revision history for this message
Christophe CHAUVET (christophe-chauvet) wrote :
Changed in openobject-server:
status: New → In Progress
Changed in openobject-server:
status: In Progress → Fix Committed
Changed in openobject-server:
status: Fix Committed → Confirmed
Revision history for this message
Christophe CHAUVET (christophe-chauvet) wrote :

Hi

This small bug was open the 25 June, When there is integrate in the next release ?

Regards,

Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :

Hello Christophe Chauvet,

One should always pass information in config file(if specified).Its not good that you pass few options from config file and others via command line arguements.

Its not only the problem of price_accuracy ,other options have the same issue.The first priority is always given to Config file.
If config file is not specified,parametres will taken from command line if specified.

I attach a patch here to make the generic and preferable situation.Your views please.

Thank you.

Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :
Revision history for this message
Christophe CHAUVET (christophe-chauvet) wrote :

Hi

You miss a point, i describe the step

First, tools/config.py create a dict with a default value, in a second time the config file was read, and in the third time, the value pass as argument are merged (or replace by the default argument) to previous read.

When you launch Open ERP as service (as Debian Ubuntu, CentOS, RedHat) it only used config file, the problem that's a default value in argument, are replace by the default one

And if you remove the default value on the parse argument, it's not a problem because the initial dict (First step) containt the default value.

I have talk about this, last week with Fabien

Regards,

Revision history for this message
Husen Daudi (husendaudi) wrote :

Hello Christophe,

We agree with your point.

But the problem occur when there is no config file exist or the file is modified with no price_accuracy option within it.
So we need default argument in each options.

Can you please try our patch and let us know?

Thanks,

Revision history for this message
Christophe CHAUVET (christophe-chauvet) wrote :

Hi hda, Jay

i've test and it doesn't work with initd service, the price accuracy in the config file is in 5 digits
But openerp-server.py invoke in the initd script have no argument and the price_accuracy in the config file was replace by the defaut value (2 digits)

If we not found a config file, the original dict (line 75 in bin/tools.config.py) assign automaticaly the 2 digits by default if not found in argument or config file.

My patch was in production in 2 customers since 6 month now,
This problem concern already the Dabian and Ubuntu package.

Regards,

Revision history for this message
Sylvain Calador (sylvain-calador) wrote :

Hi,

Same problem here with smtp_port,

While trying to change smtp_port to 2525, I have found
that somes values (like smtp_port) set in server's config file
are overwritten by the default values of optparse options defined
in __init__ of class configmanager (bin/tool/config.py).

To replicate:
Environement: Ubunutu 9.10, Python 2.5.4, openobject-server revno:1902
1. place a print on the last line(289) of init in bin/tool/config.py:
     print "smtp_port=%s" % self.options['smtp_port']
2. edit a config file with smtp_port=2525
3. start the server with -c /path/to/config/file
4. You get: smtp_port=25

A fix could be to remove default values optparse options
(because default values have bean already defined at the begining of __init__)

Regards,

Sylvain CALADOR

Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

Hi hda, jay.
I've tested your patch too. And I confirm it doesn't work. Moreover it's look very strange to have two price_accuracy default value in the same file (the proposed patch by Jay doesn't address this)

In OpenERP command line arguments are prioritar and this is logic and normal.
But as there is a default value for the option --price_accuracy in the command line arguments, this value will be always being set.
For me the solution is to apply Christophe's patch which delete the value default in the command line arguments.
This patch works fine because :
- the program uses by default the price_accuracy defined at the start of the def __init__
- after, if a price_accuracy exists in the config file this new value will replace the default value
- and to finish if a new value is set with the command line arguments this value will replace the previous value.

Regards

Sébastien BEAU

Revision history for this message
Don Kirkby (donkirkby) wrote :

Here is a summary of what I found when investigating the server options defined
in server/bin/tools/config.py version 5.0.7rc2. There are 48 options in total.
Of those, 16 can only be specified on the command line, 6 can only be specified
in the configuration file, and one is calculated based on other options. There
are also 20 of those options whose command line name either uses different words
from the configuration file setting or doesn't follow the convention with
dashes and underscores. In other words, price_accuracy is just one of many
options with problems.

I propose to change the config.py code to do the following:

1. Define all 47 options that the user can set in the OptionParser and set
their default values from a dictionary.

2. Check if the configuration file was specified on the command line, through
an environment variable, or by being in an expected location.

3. If we're using a configuration file, parse it with ConfigParser and update
all the default values in the dictionary and set them on the OptionParser. Then
parse the command line again. That way the command line will always override a
configuration file. (That seems to be the convention, I checked fetchmail's
man page.)

4. Copy all the values from the OptionParser into the config.options dictionary.

5. Validate the config.options values using the existing code.

I am attaching a proof of concept script that demonstrates loading the
OptionParser and merging the results with a ConfigParser.

While I'm at it, I propose to add alternate option names to match those that
appear in the configuration file. I would not remove any existing names, so it
wouldn't break existing configurations.

Any thoughts on this approach? I'll create a branch and start working on the
changes. My primary goal is to leave the resulting values in the config.options
dictionary unchanged from the current code. I just want to make the
configuration easier to use.

Revision history for this message
Don Kirkby (donkirkby) wrote :

I made most of my proposed changes and created a merge proposal. It's linked to this bug, but so far it's received no responses. Last week, I added three more review requests for people who had touched the configuration code while working on 5.0.9.

Revision history for this message
Christophe Simonis (OpenERP) (kangol) wrote :

Fixed by:
  5.0: 2117 <email address hidden>
  6.0: 2710 <email address hidden>

Revision history for this message
Jay Vora (Serpent Consulting Services) (jayvora) wrote :

Thanks Christophe (s).

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.