fix(hybrid-cloud): Adds defaults to provisioning model fields, lost_password_hash model #74766
Conversation
…assword_hash model Also fixes test type mismatches.
github-actions
bot
added
the
Scope: Backend
| @@ -13,7 +16,7 @@ class RpcLostPasswordHash(RpcModel): | |||
| id: int = -1 | |||
| user_id: int = -1 | |||
| hash: str = "" | |||
| date_added = datetime.datetime | |||
| date_added: datetime = Field(default_factory=timezone.now) | |||
There was a problem hiding this comment.
I'm not super sure we want this to be the default. I could also remove this entirely and just assume the caller needs to provide one, but I this was failing tests for whatever reason 🤔
There was a problem hiding this comment.
I think it's a reasonable default. Forcing the caller to provide one is reasonable in tests, but not in production, due to the reality that there will be (increasing) version drift between callers and receivers.
There was a problem hiding this comment.
Whatever default does it exist, it should be safe for the purpose of backwards compatibility, which depends on the use case.
There was a problem hiding this comment.
This default seems fine to me. Looking at the existing usage, we only create RpcLostPasswordHash by serializing an ORM object, which would have date_added set as well.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #74766 +/- ##
===========================================
+ Coverage 57.09% 78.15% +21.06%
===========================================
Files 6721 6731 +10
Lines 299870 300259 +389
Branches 51594 51647 +53
===========================================
+ Hits 171210 234669 +63459
+ Misses 123886 59267 -64619
- Partials 4774 6323 +1549
|
| @@ -13,7 +16,7 @@ class RpcLostPasswordHash(RpcModel): | |||
| id: int = -1 | |||
| user_id: int = -1 | |||
| hash: str = "" | |||
| date_added = datetime.datetime | |||
| date_added: datetime = Field(default_factory=timezone.now) | |||
There was a problem hiding this comment.
This default seems fine to me. Looking at the existing usage, we only create RpcLostPasswordHash by serializing an ORM object, which would have date_added set as well.
GabeVillalobos
added
the
Trigger: Revert
|
PR reverted: bb82492 |
…assword_hash model (#74766)
Fixes a few issues that crop up as errors in the Pydantic upgrade work: