Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.The attached patch to currency_api.module does two things:
1) Sets "Log all currency exchange requests and errors to watchdog" to be FALSE by default instead of TRUE. Why? Because logging every time the conversion function is called is very intrusive, generating thousands of entries an hour on my website. It is especially unneeded when the conversion factors are taken from the cached values in the database.
2) Makes minor changes to conform to Drupal coding standards. All that was changed is: spaces were removed in various places, and a newline was added in one place.
| Comment | File | Size | Author |
|---|---|---|---|
| currency_api.patch.gz | 1.18 KB | tr |
Comments
Comment #1
kbahey commentedI committed the second part of the patch. Thanks for that.
The first part is not a good change to have. There are hundreds of sites that use the module already and their default is to log this info. Changing the default for these sites is not good practice. You can always change it for your own site in the settings.
Comment #2
tr commentedHow about I provide a patch that changes how things are logged? Right now, it's all or nothing. I think the way it should work is that errors should *always* get logged, regardless of setting. Also, currency lookups from Yahoo should get logged if the checkbox is checked, but lookups from the database *shouldn't* get logged. That would drastically reduce the log frequency yet not lose any information.
My use case is an Ubercart store, where one customer browsing the catalog can easily see hundreds of prices per minute, all of which are logged by default. But after the first currency conversion, all of the exchange rates are the same because there is only one lookup from Yahoo then the subsequent conversion factors are found in the database. While it's important to have a record of the errors, and it's important for some sites to have a record when new rates are downloaded from Yahoo, I don't see any value in logging when the rates are retrieved from the database.
Comment #3
kbahey commentedYes. Makes more sense now.
Please open a new issue for it.
Please make the patch against a cvs checkout of the DRUPAL-5 branch, not local files.
Also, please make sure you create a DRUPAL-6--1 version of the patch as well.
Comment #4
tr commentedOK, will do.
Comment #5
tr commentedHmm, I'm confused now...
checking out HEAD gives me rev 1.4.2.11 2008/07/10,
checking out DRUPAL-5 also gives me the same rev 1.4.2.11 2008/07/10,
checking out DRUPAL-6--1 also gives me the same rev 1.4.2.11 2008/07/10,
checking out DRUPAL-5--1-1, however, gives me the previous rev - 1.4.2.10 2008/06/16
So, which branches do you want patches for ?
Comment #6
kbahey commentedThe DRUPAL-6--1 has the right code. However, the code in DRUPAL-5 is wrong, since it has the D6 code.
I will get to this in about 10 days when I am back.
Please open a separate issue for it in about a week.
Thanks