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

feat(gatsby-source-shopify): Add query runtime warning for CI environments #36142

Merged
merged 2 commits into from
Jul 29, 2022
Merged

feat(gatsby-source-shopify): Add query runtime warning for CI environments #36142

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7d2d16
Add query runtime warning for CI environments
imjoshin Jul 15, 2022
ab2b338
Make timer better
imjoshin Jul 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions packages/gatsby-source-shopify/src/create-operations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ export function createOperations(
}

async function finishLastOperation(): Promise<void> {
const queryStartTime = Date.now()
let lastWarningTime = queryStartTime
const baseWarningInterval = 60 * 1000
let warningInterval = baseWarningInterval
let warningCount = 0

let { currentBulkOperation } = await currentOperation()
if (currentBulkOperation && currentBulkOperation.id) {
if (!finishedStatuses.includes(currentBulkOperation.status)) {
Expand All @@ -68,6 +74,28 @@ export function createOperations(
while (!finishedStatuses.includes(currentBulkOperation.status)) {
await new Promise(resolve => setTimeout(resolve, 1000))
currentBulkOperation = (await currentOperation()).currentBulkOperation

// add warning for CI environments
Copy link
Contributor

Choose a reason for hiding this comment

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

@j0sh77 aren't there activities in Gatsby Cloud as well? What's different about CI vs local?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be honest, I'm not sure what qualifies to be shown in Gatsby Cloud or not. I'm not seeing anything around these long-running queries in Cloud currently. The site I'm testing with definitely has a long running query that I can reproduce locally.

@LekoArts would you happen to know if there's other qualifies an activity to show up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the activityTimer call needs an id or parentSpan to show up in the UI? 🤔 For example this activity timer shows up there https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/commands/build.ts#L358-L363

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that could be. I think this warning is still a valuable add outside of the activity timer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have a screenshot of what this looks like on Gatsby Cloud? Just want to make sure it doesn't look super messy in the structured logs view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also added a slowdown. After the first few alerts, it starts increasing the time in which it warns.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I mean in the structured logs view. But since it's slowing down over time now it should be good :) I was just worried about getting a super long page in the structured view

if (
process.env.CI &&
Date.now() > lastWarningTime + warningInterval
) {
lastWarningTime = Date.now()
const runtime = Math.floor(
(lastWarningTime - queryStartTime) / 1000
)
gatsbyApi.reporter.warn(
`Operation ${currentBulkOperation.id} is still running after ${runtime} seconds with status "${currentBulkOperation.status}"`
)

// handle next interval, slowly increase time so we don't flood every minute
warningCount += 1
warningInterval =
warningCount <= 5
? baseWarningInterval
: baseWarningInterval * 2 * (warningCount - 5)
}

timer.setStatus(
`Polling operation ${currentBulkOperation.id} : ${currentBulkOperation.status}`
)
Expand Down