Task #2674
closedReview 0000-ruby-client-config branch
Related issues
Updated by Ward Vandewege about 10 years ago
- Assigned To changed from Ward Vandewege to Tom Clegg
Updated by Ward Vandewege about 10 years ago
- Assigned To changed from Tom Clegg to Ward Vandewege
Notes:
1. By completely skipping the reading of the config file when ARVADOS_API_HOST and ARVADOS_API_TOKEN are set in the environment, you're creating a potentially confusing situation, especially if we add future settings to that configuration file.
I think what you want to do here is not allowing the overriding of ARVADOS_API_HOST_INSECURE in the config file if ARVADOS_API_HOST is set via the environment. But everything else should still apply. And it should not matter if ARVADOS_API_TOKEN is set via the environment or via the config file or not at all.
2. this isn't quite perfect yet:
- if var and val
+ if val
Apparently it is possible to assign values to a hash key that is empty. So with this in the config file
=value
that is what would happen. I suggest
if not var.empty? and val
instead.
Updated by Ward Vandewege about 10 years ago
- Assigned To changed from Ward Vandewege to Tom Clegg
Updated by Ward Vandewege about 10 years ago
- Status changed from New to In Progress
- Start date set to 04/29/2014
Updated by Tom Clegg about 10 years ago
Ward Vandewege wrote:
Notes:
1. By completely skipping the reading of the config file when ARVADOS_API_HOST and ARVADOS_API_TOKEN are set in the environment, you're creating a potentially confusing situation, especially if we add future settings to that configuration file.
Added a comment. (Currently all other settings are ignored anyway.)
I think what you want to do here is not allowing the overriding of ARVADOS_API_HOST_INSECURE in the config file if ARVADOS_API_HOST is set via the environment. But everything else should still apply. And it should not matter if ARVADOS_API_TOKEN is set via the environment or via the config file or not at all.
Also wanted to avoid uselessly reading the file, since it can have no effect at this point other than spitting errors (e.g., file is unreadable, HOME is not set).
Looking at this again I noticed a "catch all exceptions, assume/report that $HOME is unset, and continue running" pattern. Kept the logic, but reported the actual error instead of the expected error.
(Also fixed NoMethodError: undefined method `debuglog' for #<Arvados:0x007f8861dd8b00>
)
2. this isn't quite perfect yet:
- if var and val
+ if valApparently it is possible to assign values to a hash key that is empty. So with this in the config file
=value
that is what would happen. I suggest
if not var.empty? and val
instead.
Fixed.
Updated by Tom Clegg about 10 years ago
- Assigned To changed from Tom Clegg to Ward Vandewege
Updated by Ward Vandewege about 10 years ago
Tom Clegg wrote:
Added a comment. (Currently all other settings are ignored anyway.)
I think what you want to do here is not allowing the overriding of ARVADOS_API_HOST_INSECURE in the config file if ARVADOS_API_HOST is set via the environment. But everything else should still apply. And it should not matter if ARVADOS_API_TOKEN is set via the environment or via the config file or not at all.
Also wanted to avoid uselessly reading the file, since it can have no effect at this point other than spitting errors (e.g., file is unreadable, HOME is not set).
As written, yeah.
Looking at this again I noticed a "catch all exceptions, assume/report that $HOME is unset, and continue running" pattern. Kept the logic, but reported the actual error instead of the expected error.
(Also fixed
NoMethodError: undefined method `debuglog' for #<Arvados:0x007f8861dd8b00>
)
Cool.
2. this isn't quite perfect yet:
- if var and val
+ if valApparently it is possible to assign values to a hash key that is empty. So with this in the config file
=value
that is what would happen. I suggest
if not var.empty? and val
instead.
Fixed.
Thanks, ready to merge.
Updated by Ward Vandewege about 10 years ago
- Status changed from In Progress to Resolved
- Remaining (hours) changed from 0.5 to 0.0