Sandbox 554 #404
Sandbox 554 #404
Conversation
sbryzak
requested review from
MatousJobanek,
xcoulon,
alexeykazakov,
rajivnathan,
ranakan19 and
mfrancisc
as code owners
api/v1alpha1/usersignup_types.go
Outdated
| // after the expiry of their trial and based on the term specific by their UserTier. This property may be used as | ||
| // a convenience to determine the amount of time an account has left before deactivation, without requiring a separate | ||
| // lookup for the UserTier and subsequent calculation. It is managed by the Deactivation controller in the host operator. | ||
| DeactivationTimestamp string `json:"deactivationTimestamp,omitempty"` |
There was a problem hiding this comment.
shouldn't it be stored in one of the status.conditions, instead? We already add a status condition when a user is activated or banned, so why using something different for deactivations?
There was a problem hiding this comment.
ah never mind, this is the expected time of deactivation, not the actual one (ie, after the user was deactivated).
There was a problem hiding this comment.
so this field will need to be updated when a user is moved to another tier.
There was a problem hiding this comment.
Let's maybe rename it to ExpectedTimeOfDeactivation or something like that to make it clear.
There was a problem hiding this comment.
Also, could you please explain why you decided to use string? what is the expected format/content?
Why don't you use metav1.Time as we use in conditions?
There was a problem hiding this comment.
I agree with @MatousJobanek. Something like ExpectedTimeOfDeactivation with metav1.Time type would look better.
There was a problem hiding this comment.
I've renamed it to ScheduledDeactivationTimestamp, and also modified the type to use metav1.Time instead of a string to bring it inline with the rest of the API (before I saw these comments, so at least we were all on the same page ;).
|
Description
Introduce a new property to the UserSignup.Status which stores the timestamp of the user's deactivation. Related PR: codeready-toolchain/registration-service#411
Checks
Did you run
make generatetarget? yesDid
make generatechange anything in other projects (host-operator, member-operator)? yesIn case of new CRD, did you the following? n/a
resources/setup/roles/host.yamlin the sandbox-sre repositoryPROJECTfile: https://github.com/codeready-toolchain/host-operator/blob/master/PROJECTCSVfile: https://github.com/codeready-toolchain/host-operator/blob/master/config/manifests/bases/host-operator.clusterserviceversion.yamlIn case other projects are changed, please provides PR links.