Conversation
|
Strong submission, pipeline output is correct, tests pass, and Azure upload looks good! 4/5 |
| if self.price < 0 : | ||
| return ValueError(f"Invalid price {self.price} the price can not be negative!") | ||
| if not self.product_name.strip(): | ||
| return ValueError("product name can not empty or has a whitespace!") No newline at end of file |
There was a problem hiding this comment.
return ValueError(...) does not stop execution or reject bad data, it just returns a ValueError object that gets thrown away. Nothing ever raises. Replace return with raise.
| raise NotImplementedError("Implement _required: see TODO 2 in config.py") | ||
|
|
||
| value = os.environ.get(name) | ||
| if value is None : |
There was a problem hiding this comment.
Small edge case: os.environ.get(name) returns "" (empty string) for a set-but-empty variable, not None. Your check if value is None would let an empty path through.
Safer pattern would be:
value = os.environ.get(name)
if not value:
raise ValueError(f"Missing environment variable {name}. See .env.example")
| if rows is None: | ||
| return[] |
There was a problem hiding this comment.
The if rows is None: return [] guard is fine after your debugging experience. Your upstream function returned None instead of a list. Fixing the source is often better than guarding everywhere, but both approaches are valid to discuss.
2 participants
No description provided.