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

Slackbot notification builder ignores the time out of the build is monitoring #165

Open
juandelgado opened this issue Jan 11, 2019 · 2 comments

Comments

@juandelgado
Copy link

Hi there : )

When triggering the secondary build to monitor the state of the main build, the timeout of the main build is ignored.

So even if in my cloudbuild.yaml I set up a build time out of 900s, if my build happens to go over 600s no notification will be sent because the monitoring build will time out after 600s.

I've quickly hacked this in trigger.go hardcoding my own time out when creating the trigger:

		Tags: []string{"slackbot"},
		Timeout: "900s", // <--- I added this

Ideally Slackbot should pull the timeout information from the Cloud Build API in order to use the same or maybe slightly higher. But I haven't checked that this is possible and my Go knowledge is non-existent!

Thanks,

Juan

@nof20
Copy link
Contributor

nof20 commented Jan 12, 2019

Ooh, good idea! Your hack looks good to me (so your Go is good enough!)

One slight problem: the monitoring build - including its timeout - must be submitted before it can poll the Cloud Build API and get the timeout of the main build.

We could add some more blurb into the README, giving people a heads up that builds with extended timeout need to have a matching timeout added there too. It's not a great solution, but then at least it wouldn't come as a surprise. What do you think?

@juandelgado
Copy link
Author

juandelgado commented Jan 14, 2019

the monitoring build - including its timeout - must be submitted before it can poll

That makes sense, but I was guessing this would be work done by the main build, right at the time it creates the monitoring build. Again, not sure if the main build can query itself for the time out data.

We could add some more blurb into the README, giving people a heads up that builds with extended timeout need to have a matching timeout added there too

Yeah, definitely mention it on the README so it doesn't surprise people.

Also, and this caught me by surprise too, isn't the monitoring build effectively doubling up the usage of Cloud Build? This has cost implications right? There might not be another way around it, since it doesn't look like Cloud Build has "catch all" steps that get executed regardless of success / failure, but it would be good to document it.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants