kvs: support non-environment variable mechanism to choose namespace #1328
Conversation
To many libkvs functions, add input check for NULL flux handle and other bad inputs. Add unit tests appropriately.
Support new functions flux_kvs_set_namespace() and flux_kvs_get_namespace() functions, which set a namespace for a specific flux_t handle. This allows an easier interface for selecting a namespace other than setting the FLUX_KVS_NAMESPACE environment variable. The value set in the handle takes precedence over the value specified in the environment variable. Adjust functions to call flux_kvs_get_namespace() instead of private get_kvs_namespace(). As a side consequence, kvs_private.h is no longer needed as get_kvs_namespace() is removed, so remove this file. Add unit tests & manpages for new functions.
|
Just had a quick peek and looks good. The *-b, --bootstrap*='METHOD'::
Select the flux bootstrap method. Possible values of 'METHOD' are:Since the other options are subcommand-specific, should that section be renamed to COMMON OPTIONS maybe? I'd say the part in the intro about namespace consistency can be dropped, since it follows the same pattern as other KVS operations. If we do need it, maybe it could go in a new section on consistency below the subcommand descriptions? Seems a bit detailed for the intro. |
chu11
force-pushed
the
kvsnamespaceset
branch
from
da059f5 to
94108de
Compare
|
just pushed changes per your suggestions. Since the changes in the doc for the flux-kvs option was so tiny, I went ahead and squashed it. |
|
Restarted one builder that seemed hung right after the python tests (assuming that's not related to this PR). The chainlint build is failing the namespace sharness test in the same way that mine was failing in #1320. Not sure if the same background process issue is in play here as well - I didn't spot it if so, but I didn't look too hard. |
Inform callers that flux_kvs_set_namespace() is an alternate mechanism to using a different namespace.
|
It ends up with my changes, I forgot a |
chu11
force-pushed
the
kvsnamespaceset
branch
from
94108de to
9d84a85
Compare
Clean up all kvs tests to use --namespace option in flux kvs, instead of setting the environment variable. Remove now unnecessary helper functions throughout.
Add tests to ensure priority ordring of how you specify a namespace works as expected.
Update test so that fence_namespace_remove binary takes a namespace as a parameter.
chu11
force-pushed
the
kvsnamespaceset
branch
from
9d84a85 to
cfdc0ce
Compare
|
travis failed a unit test. Ended up I accidentally deleted a line in a test file. just re-pushed, |
Codecov Report
@@ Coverage Diff @@
## master #1328 +/- ##
==========================================
+ Coverage 78.2% 78.23% +0.03%
==========================================
Files 156 156
Lines 28291 28315 +24
==========================================
+ Hits 22124 22153 +29
+ Misses 6167 6162 -5
|
|
Yay! Ready? |
|
yup |
Previously, the only way to set/use a non-default KVS namespace was through the environment variable
FLUX_KVS_NAMESPACE. This is inconvenient. This PR adds aflux_kvs_set_namespace()andflux_kvs_get_namespace()set of functions, which allows a namespace to be set for a particular flux handle.This allows a simple
--namespaceoption to be added to theflux-kvscommand.This is a far more simple way to specify a namespace. The environment variable option is still there, but using the functions/kvs option takes precedence over the environment variable.
This option allows us to clean up the unit tests a ton, as there was no need to constantly set/unset
FLUX_KVS_NAMESPACEeverywhere.