Use Config parameters instead of a Config setter to set a Profile on a Config object. #5539
Conversation
kounelisagis
requested review from
sgillies,
ypatia,
ihnorton,
rroelke and
bekadavis9
| profile2.save(); | ||
|
|
||
| // Set the environment variable to select profile1 | ||
| setenv_local("TILEDB_PROFILE_NAME", profile1_name.c_str()); |
There was a problem hiding this comment.
@kounelisagis was there going to be env var for the profiles location/directory too?
There was a problem hiding this comment.
TILEDB_PROFILE_DIR works out of the box with the current implementation of Config::get. This test sets the profile directory directly in the config using config["profile_dir"] = profile_dir.
There was a problem hiding this comment.
The primary motivation for this change is to enable setting Profile details via environment variables as an alternative to configuring them through code.
Somewhere along the lines you decided against adding additional code to specially handle TILEDB_PROFILE_NAME for doing the profile name. How come? To someone who has only reviewed some of the code that seems like a lighter alternative.
The profiles introduce another level of hierarchy into the config process so it feels a little odd to have them expressed in the same way as other config parameters.
That said, this looks like a correct implementation of the chosen design - just a few small comments.
…o-set-a-profile-on-a-config
kounelisagis
deleted the
agis/use-config-variable-instead-of-setter-to-set-a-profile-on-a-config
branch
This PR removes the
Config::set_profilemethod along with its corresponding C and C++ APIs, changing the logic so that users can specify a Profile directly on a Config object to retrieve itsrest.*parameters. Instead of using the setter method, the new approach is to pass the Profile details as normal config parameters:"profile_name"and"profile_dir".The primary motivation for this change is to enable setting Profile details via environment variables as an alternative to configuring them through code. This approach leverages the existing behavior of
Config::get_from_config_or_fallback, which reads environment variables and treats them as standard config parameters. As a result, users can now use theTILEDB_PROFILE_NAMEandTILEDB_PROFILE_DIRenvironment variables, improving the overall user experience - especially for those who frequently switch/test across different internal clusters.Closes CORE-235
TYPE: IMPROVEMENT
DESC: Use config parameters instead of a config setter to set a profile on a config object.