Skip to content

[db] Move db models to go-db module #14770

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

Merged
merged 1 commit into from
Nov 21, 2022
Merged

[db] Move db models to go-db module #14770

merged 1 commit into from
Nov 21, 2022

Conversation

laushinka
Copy link
Contributor

@laushinka laushinka commented Nov 17, 2022

Description

Moves DB models to components/gitpod-db/go, so that they can be used by public-api but also usage.

Changes performed in this PR:

  • Move all DB models from components/usage/pkg/db into components/gitpod-db/go
  • Created a BUILD.yaml for go
  • Update packages correspondingly
  • Update imports correspondingly

Related Issue(s)

New attempt at #14731
Requirement for #14602

How to test

Build and unit tests

Release Notes

NONE

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@easyCZ
Copy link
Member

easyCZ commented Nov 18, 2022

@mads-hartmann We'll still need the leeway change to get this PR through. Do you have a ref for the fix so we can track, please?

@easyCZ easyCZ force-pushed the lau/move-db-models-godb branch from c38077e to aa94e72 Compare November 18, 2022 11:18
@laushinka laushinka marked this pull request as ready for review November 18, 2022 11:27
@laushinka laushinka requested review from a team November 18, 2022 11:27
@github-actions github-actions bot added team: SID team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Nov 18, 2022
Copy link
Contributor

@andrew-farries andrew-farries left a comment

Choose a reason for hiding this comment

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

👍 Changes look mostly mechanical and I paired with @laushinka for some of this yesterday.

/hold for one small change.

@@ -28,16 +28,16 @@ func ConnectForTests(t *testing.T) *gorm.DB {
return conn
}

// These are static connection details for tests, started by `leeway components/usage:init-testdb`.
// These are static connection details for tests, started by `leeway components/gitpod-db:init-testdb`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// These are static connection details for tests, started by `leeway components/gitpod-db:init-testdb`.
// These are static connection details for tests, started by `leeway build components/gitpod-db/go:init-testdb`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@mads-hartmann
Copy link
Contributor

mads-hartmann commented Nov 18, 2022

@laushinka @easyCZ It looks like the build is fine without the Leeway change now? ☺️ I opened a PR to fix it anyway as it was indeed a bug in Leeway - @andrew-farries have a look at the PR description if you want to learn more about ephemeral packages.

@andrew-farries
Copy link
Contributor

Thanks for the Notion doc @mads-hartmann.

If ephemeral packages are something that we intend to keep using, perhaps your PR (or a followup) could add documentation about them to leeway's README.md?

@mads-hartmann
Copy link
Contributor

mads-hartmann commented Nov 18, 2022

If ephemeral packages are something that we intend to keep using, perhaps your PR (or a followup) could add documentation about them to leeway's README.md?

I will try to get clarity around the future of ephemeral packages with the team and Chris. If we keep them around I will for sure update the documentation 🧡

@easyCZ
Copy link
Member

easyCZ commented Nov 18, 2022

@mads-hartmann I guess it's not really needed. Which raises the Q why was it needed before? The changes here are the same as in the original PR.

@mads-hartmann
Copy link
Contributor

mads-hartmann commented Nov 18, 2022

@easyCZ I think you're seeing the problem I'm describing in the PR. Looking at the logs of one of the failed jobs (job 12) you can see that it didn't need to rebuild components/common-go:lib(cached remotely (downloaded) components/common-go:lib) which means it wouldn't "build" the ephemeral package init-testdb. Looking at the successful build in this branch (job 24) you can see it does need to build components/gitpod-db/go:lib so the ephemeral package components/gitpod-db:dbtest-init will be built too, which means that any package that depends on components/gitpod-db/go:lib will built successfully.

So I think without the PR we'll see flaky builds depending on whether or not Leeway needs to build components/gitpod-db/go:lib or not. I'm not 100% sure on this, but it's my best guess ☺️

@easyCZ
Copy link
Member

easyCZ commented Nov 21, 2022

@mads-hartmann Looks like you've managed to land gitpod-io/leeway#121, what changes do we need to make to this PR to use it?

@mads-hartmann
Copy link
Contributor

mads-hartmann commented Nov 21, 2022

@easyCZ I'll open a PR to bump the Leeway version everywhere now. I'll ping you as soon as it's ready, and once that's in we can merge this PR ☺️

@mads-hartmann mads-hartmann mentioned this pull request Nov 21, 2022
4 tasks
@mads-hartmann
Copy link
Contributor

mads-hartmann commented Nov 21, 2022

@easyCZ PR here but the main build is broken since a few hours ago, so I'm going down that rabbit hole first ☺️

@mads-hartmann
Copy link
Contributor

@easyCZ It's in main :shipit:

@easyCZ easyCZ force-pushed the lau/move-db-models-godb branch from 97ab7e2 to 95fe93e Compare November 21, 2022 10:39
@easyCZ easyCZ force-pushed the lau/move-db-models-godb branch from 95fe93e to b4fc228 Compare November 21, 2022 10:41
@easyCZ
Copy link
Member

easyCZ commented Nov 21, 2022

/werft run

👍 started the job as gitpod-build-lau-move-db-models-godb.27
(with .werft/ from main)

@easyCZ
Copy link
Member

easyCZ commented Nov 21, 2022

/unhold

@roboquat roboquat merged commit ee08e78 into main Nov 21, 2022
@roboquat roboquat deleted the lau/move-db-models-godb branch November 21, 2022 11:37
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Nov 21, 2022
@roboquat roboquat added the deployed: workspace Workspace team change is running in production label Dec 7, 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: workspace Workspace team change is running in production release-note-none size/XXL team: SID team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants