Add feature to save each key in different data bag item #246
Conversation
Signed-off-by: Robert Veznaver <r.veznaver@criteo.com>
Signed-off-by: Robert Veznaver <r.veznaver@criteo.com>
| return (ckey ? true : false) unless ckey.nil? | ||
| # check if the key is saved in sparse mode | ||
| spath = "#{@raw_data["id"]}_key_#{key}" | ||
| skey = if Chef::Config[:solo_legacy_mode] |
There was a problem hiding this comment.
this block of code should be factorized (including the spatch computation)
There was a problem hiding this comment.
Good point. Will try to fix once tests pass.
| @raw_data.keys.include?(key) | ||
| # check if the key is in the write-back cache | ||
| ckey = @cache[key] | ||
| return (ckey ? true : false) unless ckey.nil? |
There was a problem hiding this comment.
OPT:
(if you really want to return true value)
return !!ckey if ckey
or if you just want to return true-ish value
return ckey if ckey
There was a problem hiding this comment.
Did you mean to write !!ckey unless ckey.nil? ?
Rubocop doesn't like that :(
| @cache.each do |key, val| | ||
| spath = "#{@raw_data["id"]}_key_#{key}" | ||
| # delete across all modes on key deletion | ||
| if val == false |
There was a problem hiding this comment.
nil and false do not have the same meaning in the write-back cache
| begin | ||
| Chef::DataBagItem.destroy(@data_bag, spath) | ||
| rescue Net::HTTPServerException => http_error | ||
| nil if http_error.response.code == "404" |
There was a problem hiding this comment.
I don't understand why you specify nil here since this can't be a returned value
There was a problem hiding this comment.
Because I copied it without checking :) Will fix.
Signed-off-by: Robert Veznaver <r.veznaver@criteo.com>
Signed-off-by: Robert Veznaver <r.veznaver@criteo.com>
|
pinging @thommay for input |
|
I think the major question I have here is does it still make sense for ItemKeys to inherit from DataBagItem? If it does not, does the code get simpler? |
|
@thommay I had the same question, but have chosen to err on the side of code consistency. IMO, if ItemKeys were to be a standalone, hash-like class, then Items should be as well, but that would require a more thorough refactor. |
|
I think ItemKeys could change without needing to change Item (which is still fundamentally a DBI at the end of the day). |
|
So is ItemKeys :) |
|
OK. I'm going to merge this as is, since it's clearly better. Then I'm going to do another release candidate. |
|
Thanks, @rveznaver for the great PR. |
|
Thanks, I'll soon start working on the next step: adding knife support and sparse mode cucumber tests. |
As described in #237.