Fix for cache bug in s3fs #812
Conversation
| @@ -575,6 +575,10 @@ def _read_buckets_cache_file(cache_file): | |||
| with salt.utils.files.fopen(cache_file, 'rb') as fp_: | |||
| try: | |||
| data = pickle.load(fp_) | |||
| # check for 'corrupted' cache data ex: {u'base':[]} | |||
| if not list(filter(None, data.values())): | |||
There was a problem hiding this comment.
list(filter()) please.
Isn't it enough to say
if not data.get('base'):
data = None
Or at the very least, something like
if not any(data.values()):
data = None
It's a nit-pick for sure, but ...
There was a problem hiding this comment.
Last one will work just fine I believe. First one won't since saltenv won't always be base and it doesn't cover the use case of having multiple saltenvs {u'base': [], u'testing':[{u'somedict':[]}]}
There was a problem hiding this comment.
I really hope people aren't using saltenv in their hubble ... it doesn't work very well in salt. It sure as heck won't work in hubble. I think the whole reference to 'base' is vestigial in this context. But whatever. As long as we're not using filter() and forcing our iterator down to a list(), we'll be fine.
|
@daveed12, for documentation sake, any comments on how this was tested and some elaboration on what situation does this fix? |
|
To be merged to 3.0 and 4.0. |
That's the BackFeature label in this case. applied. |
|
Oh, wait, this is against develop... It should be against 3.0 branch... |
|
So I put BackFeature (to 4.0) and To Backport (to 3.0) |
jettero
added
Backport Complete
Forwardport Complete
and removed
To Backport
BackFeature
Handles cached data correctly for s3fs if non 2XX response occurs.