Better handling of empty/missing roster in config #112
Conversation
LinuxMercedes
force-pushed
the
pr/better-roster-handling
branch
from
5c7acb8
to
24acb14
Compare
assigner/config.py
Outdated
| return self.data[key] | ||
| raise e | ||
| # Fill in a blank roster if needed | ||
| if key == "roster" and (key not in self.data or self.data[key] is None): |
There was a problem hiding this comment.
Is self.data a property? I can't remember. My intuition here is to tell you to use self.data.get(key, None) or self.data.get(key) which defaults to None (or it did in Python 2).
There was a problem hiding this comment.
Calling self.data.get(key, []) doesn't set self.data[key] = [], so any modifications to the empty roster list returned wouldn't actually show up in the config.
Example:
>>> data = {}
>>> l = data.get("bob", [])
>>> l
[]
>>> data
{}
>>> l.append(5)
>>> l
[5]
>>> data
{}
There was a problem hiding this comment.
Ohh, I see what you mean. Yeah, I'll change the condition--yours is a lot nicer. ❤️
| if key == "roster": | ||
| self.data[key] = [] | ||
| return self.data[key] | ||
| raise e |
There was a problem hiding this comment.
This is more Pythonic. Without reading into it more, why are we nuking it?
There was a problem hiding this comment.
This check fails to catch the self.data['roster'] is None case.
LinuxMercedes
force-pushed
the
pr/better-roster-handling
branch
from
24acb14
to
c93b9df
Compare
LinuxMercedes
force-pushed
the
pr/better-roster-handling
branch
from
c93b9df
to
25ff11d
Compare
No description provided.