Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[db] Add transformer to map MySQL BIGINT to Number, use for Prebuild.statusVersion #9152

Merged
merged 1 commit into from
Apr 7, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Apr 6, 2022

Description

MySQL serializes the BIGINT type as a string, and without an explicit setting in TypeORM (on global level), it is retrieved as a string.

This PR adds a transformer which converts the retrieved BIGINT as string into a number on the underlying DB object representation.

Before

{
    "component": "ws-manager-bridge",
    "severity": "INFO",
    "time": "2022-04-06T13:39:42.867Z",
    "message": "Found prebuild record in database.",
    "payload": {
        "id": "f42f95e7-7c96-4fc4-9513-e1171b84d176",
        "cloneURL": "https://gitlab.com/gitpod-milan/gitpod-large-image.git",
        "commit": "d741ef690a211f5a4157f3edc685c984b3b4b1d1",
        "projectId": "46fb2b28-9a46-4c4b-8d3b-a51125edb690",
        "branch": "main",
        "state": "building",
        "creationTime": "2022-04-06T13:39:39.000Z",
        "buildWorkspaceId": "gitpodmilan-gitpodlargei-32ivsk09l19",
        "statusVersion": "108085404106752"
    }
}

After

{
    "component": "ws-manager-bridge",
    "severity": "INFO",
    "time": "2022-04-06T19:54:37.689Z",
    "message": "Found prebuild record in database.",
    "payload": {
        "id": "9a78ec6d-cc5e-4b32-be92-2243a4cb05a3",
        "cloneURL": "https://gitlab.com/gitpod-milan/gitpod-large-image.git",
        "commit": "753d6ee903d63c3707714f7ba79f3a5aa9705830",
        "projectId": "3602dab2-5849-4e86-b17c-2038b47bc225",
        "branch": "pass",
        "state": "building",
        "creationTime": "2022-04-06T19:54:32.000Z",
        "buildWorkspaceId": "gitpodmilan-gitpodlargei-p25esupv5zm",
        "statusVersion": 108086878208000
    }
}

How to test

  1. Start a prebuild
  2. Check ws-manager-logs for a log with "message": "Found prebuild record in database.",, observe the statusVersion is a number, rather than a string.

Release Notes

Transformer for MySQL BIGINT type to JS number

Documentation

/uncc
/hold

@@ -73,6 +73,7 @@ export class DBPrebuiltWorkspace implements PrebuiltWorkspace {
// statusVersion must only be set by controller/observer.
@Column({
default: 0,
type: "bigint",
Copy link
Member

@geropl geropl Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@easyCZ we could leave it an bigint/number and use a transformer to do the conversion from/to DB. see transformers.ts (or typeorm.ts? 🤔 )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea! Let me give that a go.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This worked! Thanks a lot for suggesting it, I really wasn't happy with the solution I had originally.

@easyCZ
Copy link
Member Author

easyCZ commented Apr 6, 2022

/werft run with-clean-slate=true

👍 started the job as gitpod-build-mp-status-version-string.6

@easyCZ
Copy link
Member Author

easyCZ commented Apr 6, 2022

/werft run with-clean-slate=true

👍 started the job as gitpod-build-mp-status-version-string.8

@easyCZ easyCZ changed the title [db] Type Prebuild.statusVersion as string due to bigint db represenation [db] Add transformer to map MySQL BIGINT to Number, use for Prebuild.statusVersion Apr 6, 2022
@easyCZ easyCZ requested a review from geropl April 7, 2022 06:39
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@easyCZ
Copy link
Member Author

easyCZ commented Apr 7, 2022

/unhold

@roboquat roboquat merged commit 55933f3 into main Apr 7, 2022
@roboquat roboquat deleted the mp/status-version-string branch April 7, 2022 13:12
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Apr 12, 2022
@roboquat roboquat added the deployed Change is completely running in production label Apr 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/M team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants