Skip to content

[ci breaking-change] feat: detect breaking jetstream images#3063

Open
mikewilli wants to merge 7 commits intomainfrom
3062-breaking-changes-note
Open

[ci breaking-change] feat: detect breaking jetstream images#3063
mikewilli wants to merge 7 commits intomainfrom
3062-breaking-changes-note

Conversation

@mikewilli
Copy link
Contributor

  • adds a check for any images with the breaking tag that are newer than otherwise would've been retrieved for an experiment
  • adds a CI check to warn when breaking changes might be present, and what to do if so
  • adds CI workflow to add the breaking tag to the image if requested

fixes #3062

@mikewilli mikewilli force-pushed the 3062-breaking-changes-note branch from dcaf0cc to 63de321 Compare March 25, 2026 21:00
@mikewilli mikewilli changed the title feat: detect breaking jetstream images [ci breaking-change] feat: detect breaking jetstream images Mar 25, 2026
Comment on lines 115 to 118
run: docker build . -t jetstream:latest -t jetstream:breaking
- name: Push Docker image to GAR
uses: mozilla-it/deploy-actions/docker-push@v3
with:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if you already checked if publishing the two tags works? I had some problems with the newer version of docker-push where only one tagged image could be published at a time resulting in multiple tasks: https://github.com/mozilla/lookml-generator/blob/efc47dfa6a0670028dd9bb3bd61fb2f90de9041a/.github/workflows/build-and-deploy.yml#L289-L307

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good to know, thanks! I actually had it doing this separately at first but then was hoping it would work this way and was about to test it. I'll test anyway, but will probably have to split it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok confirmed that this does not work, I'll split it out.

if breaking_image:
breaking_time = breaking_image.upload_time
# see note below about mypy ignore here
if last_updated and last_updated < breaking_time: # type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess in theory if a breaking image is pushed around the time an analysis run is triggered/running (which uses the previous image version), the created tables for new experiments might end up having a creation timestamp < breaking_time. I don't expect that this is likely to happen, so probably not worth thinking too much about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm I think you mean this scenario?

  1. analysis starts but has not written to any tables
  2. breaking image is pushed
  3. analysis writes to a table
    This is ok because a subsequent run should pick up the breaking image using the existing logic in _image_for_date (breaking image upload time is the latest image at the time of the table creation). In the other direction, if the breaking image is pushed just after table creation, then this check handles that (tables created before breaking image upload, so use the breaking image).

Or am I misunderstanding your scenario?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that's the scenario I was thinking of. I was ignoring the _image_for_date, but makes sense. This shouldn't be a concern then

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

Successfully merging this pull request may close these issues.

Ability to denote breaking changes between jetstream versions

2 participants