Skip to content

TheBESKeys refactor#1305

Open
kneumiller wants to merge 7 commits intomasterfrom
thebeskeys_refactor
Open

TheBESKeys refactor#1305
kneumiller wants to merge 7 commits intomasterfrom
thebeskeys_refactor

Conversation

@kneumiller
Copy link
Copy Markdown
Contributor

Refactor multiple handlers to use TheBESKeys functions instead of the custom functions they were using.

I also added a "read_float_key()" to TheBESKeys. So far only used in the HDF5RequestHandler.

kneumiller and others added 6 commits April 1, 2026 12:21
Keep the handler overloads to avoid touching a lot of callers right now. TheBESKeys does not have a read_double_key() so I am leaving that one as is right now.
TheBESKeys::TheKeys()->get_value(key, value, found);
// 'key' holds the string value at this point if found is true
if (found) {
std::istringstream iss(value);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::istringstream iss(value);
float float_val = std::stof(value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, use std::stof() which was added to C++-14. Drop the next five lines; stof() will throw an exception. It throws std::invalid_argument if no conversion could be performed and std::out_of_range if the value is too large for a float. You could catch those and use the information to throw a BESerror in it's place, or you could just let these propagate up. Either way, remove the return default_value; here.

return (dosettrue == doset || dosetyes == doset);
}
return def_val;
return TheBESKeys::read_bool_key(key, def_val);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is get_bool_key() used in this code? I don't think so but it's hard to follow from the diff. If it's not used, remove it.

NCRequestHandler::_use_mds = TheBESKeys::read_bool_key("NC.UseMDS", false);
NCRequestHandler::_cache_entries = static_cast<unsigned int>(TheBESKeys::read_ulong_key("NC.CacheEntries", 0));
// No float option in TheBESKeys
NCRequestHandler::_cache_purge_level = get_float_key("NC.CachePurgeLevel", 0.2);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
NCRequestHandler::_cache_purge_level = get_float_key("NC.CachePurgeLevel", 0.2);
NCRequestHandler::_cache_purge_level = TheBESKeys::read_float_key("NC.CachePurgeLevel", 0.2);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, forgot to call TheBESKeys, good catch

@jgallagher59701
Copy link
Copy Markdown
Member

The tests failing in modules/functions/stare say something very odd, WRT this change:

stare/tests % more testsuite.dir/26/testsuite.log 
#                             -*- compilation -*-
26. testsuite.at:41: testing bescmd/stare_box_subset_array_0.dap.nc4.bescmd ...
./testsuite.at:41: besstandalone -c $abs_builddir/$bes_conf -i $input > test.nc
./testsuite.at:41: ncdump -k test.nc > tmp
--- /dev/null   2026-04-08 17:44:09.147999962 +0000
+++ /home/travis/build/OPENDAP/bes/modules/functions/stare/tests/testsuite.dir/at-groups/26/stderr      2026-04-08 17:53:04.687987713 +0000
@@ -0,0 +1 @@
+ncdump: test.nc: NetCDF: Unknown file format
./testsuite.at:41: exit code was 1, expected 0
26. testsuite.at:41: 26. bescmd/stare_box_subset_array_0.dap.nc4.bescmd (testsuite.at:41): FAILED (testsuite.at:41)
stare/tests % 
stare/tests % more testsuite.dir/27/testsuite.log
#                             -*- compilation -*-
27. testsuite.at:42: testing bescmd/stare_box_subset_array_1.dap.nc4.bescmd ...
./testsuite.at:42: besstandalone -c $abs_builddir/$bes_conf -i $input > test.nc
./testsuite.at:42: ncdump -k test.nc > tmp
--- /dev/null   2026-04-08 17:44:09.147999962 +0000
+++ /home/travis/build/OPENDAP/bes/modules/functions/stare/tests/testsuite.dir/at-groups/27/stderr      2026-04-08 17:53:07.011987660 +0000
@@ -0,0 +1 @@
+ncdump: test.nc: NetCDF: Unknown file format
./testsuite.at:42: exit code was 1, expected 0
27. testsuite.at:42: 27. bescmd/stare_box_subset_array_1.dap.nc4.bescmd (testsuite.at:42): FAILED (testsuite.at:42)
stare/tests % more testsuite.dir/28/testsuite.log
#                             -*- compilation -*-
28. testsuite.at:47: testing bescmd/stare_box_subset_array_tasmania.dap.nc4.bescmd ...
./testsuite.at:47: besstandalone -c $abs_builddir/$bes_conf -i $input > test.nc
./testsuite.at:47: ncdump -k test.nc > tmp
--- /dev/null   2026-04-08 17:44:09.147999962 +0000
+++ /home/travis/build/OPENDAP/bes/modules/functions/stare/tests/testsuite.dir/at-groups/28/stderr      2026-04-08 17:53:06.539987670 +0000
@@ -0,0 +1 @@
+ncdump: test.nc: NetCDF: Unknown file format
./testsuite.at:47: exit code was 1, expected 0
28. testsuite.at:47: 28. bescmd/stare_box_subset_array_tasmania.dap.nc4.bescmd (testsuite.at:47): FAILED (testsuite.at:47)
testsuite.dir/28/testsuite.log (END)

if (get_usecf())
load_config_cf_cache();

_stp_east_filename = get_beskeys("H5.STPEastFileName");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_beskeys returns a string. But the read_bool_key returns a boolean. You should use TheBESkeys::read_string_key(key,"") in this case.

load_config_cf_cache();

_stp_east_filename = get_beskeys("H5.STPEastFileName");
_stp_north_filename = get_beskeys("H5.STPNorthFileName");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

_stp_east_filename = TheBESKeys::read_bool_key("H5.STPEastFileName", false);
_stp_north_filename = TheBESKeys::read_bool_key("H5.STPNorthFileName", false);
BESDEBUG(HDF5_NAME, prolog << "END" << endl);
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above and the rest of the similar ones.

if (has_key)
_use_latlon_disk_cache = key_value;
BESDEBUG(HDF5_NAME, prolog << "H5.EnableEOSGeoCacheFile: " << (_use_latlon_disk_cache?"true":"false") << endl);
_latlon_disk_cache_size = get_uint_key("H5.Cache.latlon.size",0);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not using read_ulong_key?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got the "read_*_keys" mixed up, thank you for catching this.

HDF5RequestHandler::_lrdcache_entries = get_uint_key("H5.LargeDataMemCacheEntries", 0);
HDF5RequestHandler::_srdcache_entries = get_uint_key("H5.SmallDataMemCacheEntries", 0);
HDF5RequestHandler::_cache_purge_level = get_float_key("H5.CachePurgeLevel", 0.2F);
HDF5RequestHandler::_mdcache_entries = TheBESKeys::read_uint64_key("H5.MetaDataMemCacheEntries", 0);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using read_ulong_key? For the similar cases below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mistaken. I saw that "_mdcache_entries", "_lrdcache_entries", and "_srdcache_entries" were unsigned int, as defined below, and called "read_uint64_key". I'll switch it to "read_ulong_key"
unsigned int HDF5RequestHandler::_mdcache_entries = 500;
unsigned int HDF5RequestHandler::_lrdcache_entries = 0;
unsigned int HDF5RequestHandler::_srdcache_entries = 0;

…ys such as "read_ulong_key" and "read_string_key"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants