Do not try to deploy to fastly-staging in deploy-webapp.groovy.#234
Open
Do not try to deploy to fastly-staging in deploy-webapp.groovy.#234
Conversation
That's too late! This script doesn't get run until _after_ you say `sun: set default`. The whole point of the staging environment is to be available _before_ you say `sun: set default`, when you're doing the manual testing. Sadly, doing this in build-webapp.groovy isn't right either, since many of those get run in parallel, and there's only one fastly-staging service. If we really wanted to cram this logic in, we'd have to do it in buildmaster, before showing the "time to set default" message to the user. But that's a really awkward place to put it. We'll have to think more about what the right thing to do here. The whole fastly-staging thing isn't as useful as it might be, since it doesn't work for first smoke tests, znd's, or any number of other things we might want it to. Perhaps fixing that problem will make it obvious where this logic should move to (probably build-webapp.groovy). Issue: none Test plan: Noticing https://jenkins.khanacademy.org/job/deploy/job/deploy-webapp/20552/console
kevinb-khan
approved these changes
Oct 10, 2024
Contributor
kevinb-khan
left a comment
kevinb-khan
left a comment
There was a problem hiding this comment.
Makes sense. Hopefully things work out with it being in build-webapp.groovy.
MiguelCastillo
approved these changes
Oct 22, 2024
Member
MiguelCastillo
left a comment
MiguelCastillo
left a comment
There was a problem hiding this comment.
Yeah as we have briefly chatted about it, we have a couple of things to sort out. https://khanacademy.slack.com/archives/C096UP7D0/p1729628054920289
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
That's too late! This script doesn't get run until after you say
sun: set default. The whole point of the staging environment is tobe available before you say
sun: set default, when you're doingthe manual testing.
Sadly, doing this in build-webapp.groovy isn't right either, since
many of those get run in parallel, and there's only one fastly-staging
service. If we really wanted to cram this logic in, we'd have to do
it in buildmaster, before showing the "time to set default" message to
the user. But that's a really awkward place to put it.
We'll have to think more about what the right thing to do here. The
whole fastly-staging thing isn't as useful as it might be, since it
doesn't work for first smoke tests, znd's, or any number of other
things we might want it to. Perhaps fixing that problem will make it
obvious where this logic should move to (probably
build-webapp.groovy).
Issue: none
Test plan:
Noticing https://jenkins.khanacademy.org/job/deploy/job/deploy-webapp/20552/console