[usage] store more data for in usage entry #12736
Conversation
svenefftinge
force-pushed
the
se/usage-more-data
branch
from
249bb49 to
02a9a7e
Compare
svenefftinge
force-pushed
the
se/usage-more-data
branch
from
02a9a7e to
2802ad0
Compare
| return "d_b_user" | ||
| } | ||
|
|
||
| func NewUser(t *testing.T, user User) User { |
There was a problem hiding this comment.
It's bit of an anti-pattern to have a testing.T used in a non test package. Normally we'd pull this into a separate package - it also has the benefit of not being included in the resulting binary if it's in a different package (not referenced by "prod" code)
There was a problem hiding this comment.
it's in a testing package
There was a problem hiding this comment.
Sorry, I got confused. The User model definition should be moved out of the dbtest package into the db package
| @@ -23,7 +24,7 @@ func NewWorkspaceInstance(t *testing.T, instance db.WorkspaceInstance) db.Worksp | |||
| t.Helper() | |||
|
|
|||
| id := uuid.New() | |||
| if instance.ID.ID() != 0 { // empty value | |||
| if instance.ID != uuid.Nil { | |||
| type User struct { | ||
| ID uuid.UUID `gorm:"primary_key;column:id;type:char;size:36;"` | ||
| AvatarURL string `gorm:"column:avatarUrl;type:char;size:255;"` | ||
| Name string `gorm:"column:name;type:char;size:255;"` | ||
| FullName string `gorm:"column:fullName;type:char;size:255;"` | ||
| CreationDate db.VarcharTime `gorm:"column:creationDate;type:varchar;size:255;"` | ||
|
|
||
| // user has more field but we don't care here as they are just used in tests. | ||
| } |
There was a problem hiding this comment.
This should be in the db package, not the dbtest package
There was a problem hiding this comment.
I put it here, because we don't need this in production code, it's only used for setting up testing state in DB.
There was a problem hiding this comment.
I see. I guess I can live with this, but I'd generally define it on the db package as someone else will come along and not find the User object there so they would go and define it again
| for _, ws := range retrieved { | ||
| ids = append(ids, ws.ID) | ||
| } | ||
| retrieved := dbtest.ListWorkspaceInstancesInRange(t, conn, startOfMay, startOfJune, workspace.ID) |
There was a problem hiding this comment.
The listing methods should not be in the dbtest package, should be in the db package instead.
There was a problem hiding this comment.
It is a testing utility that requires to provide a workspaceId and filters the result on it to make tests more stable. Should I name it differently to make it clearer?
|
/werft run 👍 started the job as gitpod-build-se-usage-more-data.5 |
|
/werft run 👍 started the job as gitpod-build-se-usage-more-data.6 |
roboquat
added
deployed: webapp
Description
Fetch additional data for workspace instances and store it in d_b_usage.
Related Issue(s)
Fixes #
How to test
Release Notes
Documentation
Werft options: