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.

Sign up for GitHub

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

Jump to bottom

[usage] Fix querying workspace instances to use startedTime existence to filter #10778

Merged
merged 1 commit into from
Jun 21, 2022
Merged

[usage] Fix querying workspace instances to use startedTime existence to filter #10778

merged 1 commit into from
Jun 21, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented Jun 20, 2022 •
edited

Description

The underlying query used creationTime to validate the record should be included but in #10642 (comment) it was pointed out that should not be the case, and startedTime should be used instead.

The rest of the changes are to tests, and streamlining test object creation.

Related Issue(s)

Fixes #10642

How to test

Unit tests

Release Notes

NONE

Documentation

NONE

Werft options:

@easyCZ easyCZ marked this pull request as ready for review June 20, 2022 21:11
@easyCZ easyCZ requested a review from a team June 20, 2022 21:11
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jun 20, 2022
andrew-farries
andrew-farries reviewed Jun 21, 2022
View reviewed changes
components/usage/pkg/controller/reconciler_test.go Outdated
}

expectedRuntime := instances[0].WorkspaceRuntimeSeconds(scenarioRunTime) + instances[1].WorkspaceRuntimeSeconds(scenarioRunTime)

return Scenario{
Name: "one team with one workspace",
Name: "oen team with one workspace",
Copy link
Contributor

Choose a reason for hiding this comment

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

typo here

components/usage/pkg/db/workspace_instance.go Show resolved Hide resolved
@andrew-farries
Copy link
Contributor

The build is failing on the TestUsageReconciler_ReconcileTimeRange/oen_team_with_one_workspace test.

This was pointed out as a flaky test last week (see internal discussion).

@easyCZ
Copy link
Member Author

easyCZ commented Jun 21, 2022

The build is failing on the TestUsageReconciler_ReconcileTimeRange/oen_team_with_one_workspace test.

This was pointed out as a flaky test last week (see internal discussion).

Something is off with werft. The build it links to is actually not the latest (ID 2). Latest is https://werft.gitpod-dev.com/job/gitpod-build-mp-usage-fix-query.3/raw which passes

@easyCZ easyCZ mentioned this pull request Jun 21, 2022
Closed
@andrew-farries
Copy link
Contributor

Something is off with werft. The build it links to is actually not the latest (ID 2). Latest is https://werft.gitpod-dev.com/job/gitpod-build-mp-usage-fix-query.3/raw which passes

I think there is still an intermittent problem with that test however (happened to me once last week and to @akosyakov too).

@easyCZ
Copy link
Member Author

easyCZ commented Jun 21, 2022

/unhold

@roboquat roboquat merged commit 006dfb6 into main Jun 21, 2022
@roboquat roboquat deleted the mp/usage-fix-query branch June 21, 2022 08:20
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrew-farries andrew-farries andrew-farries approved these changes

Assignees
No one assigned
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Usage] workspace.go:68 Error 1390: Prepared statement contains too many placeholders
3 participants
@easyCZ @andrew-farries @roboquat