Fields: created & last_updated moved to Model. #3325
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Jeff,
thanks for the patch!
I think the docstring needs to be corrected for the created attribute on the Model, otherwise LGTM.
Cheers,
milan
pulpcore/pulpcore/app/models/base.py
Outdated
| @@ -11,7 +11,9 @@ class Model(models.Model): | |||
| behavior. | |||
|
|
|||
| Fields: | |||
| id: UUID ID Primary Key Field | |||
| id (models.UUIDField): UUID ID Primary Key Field | |||
| create (models.DateTimeField): Created timestamp UTC. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/create/created/
| @@ -11,7 +11,9 @@ class Model(models.Model): | |||
| behavior. | |||
|
|
|||
| Fields: | |||
| id: UUID ID Primary Key Field | |||
| id (models.UUIDField): UUID ID Primary Key Field | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: unrelated but OK
| @@ -271,7 +266,7 @@ def content(self): | |||
| >>> repository_version = ... | |||
| >>> | |||
| >>> for content in repository_version.content: | |||
| >>> content = content.case() # optional downcast. | |||
| >>> content = content.cast() # optional downcast. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: unrelated, but OK
| @@ -20,10 +22,9 @@ class Model(models.Model): | |||
| * https://www.postgresql.org/docs/current/static/datatype-uuid.html | |||
|
|
|||
| """ | |||
| # ...we have zero interest in using a mongo-specific datatype (ObjectId) as | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: unrelated but OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. The docs builder is failing due to changes in sphinx and not because of changes in this PR.
Related to https://pulp.plan.io/issues/3337, the FileContent needed to be ordered by a
createdfield. This seemed generally useful for other plugins for the same reason. Then, considered that acreatedtimestamp would be useful on most models. So, why not add it toModel. Along the same line of thinking, why not addlast_updatedwhile we're at it. Should be generally useful for filtering/search and troubleshooting.There are some concerns within the django community regarding auto_now and auto_now_add but I suggest we make this change independently of that. We should be revisited and fix on all models as needed.