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

Quick fix for accidentally *fixing* @build deps #2889

Merged
merged 1 commit into from
Feb 22, 2025

Conversation

freider
Copy link
Contributor

@freider freider commented Feb 21, 2025

In version v0.73.55 we accidentally fixed one half of a bug that previously would not add dependency objects to @build methods of classes.

Fixing the other half of the bug, which is that the dependencies are not included in the app layout as provided by the workers seems a bit more involved, so for now this just makes the two dependency lists consistent (but still incorrect, causing image.imports to not work properly in @build functions)


Check these boxes or delete any item (or this section) if not relevant for this PR.

  • Client+Server: this change is compatible with old servers
  • Client forward compatibility: this change ensures client can accept data intended for later versions of itself

Note on protobuf: protobuf message changes in one place may have impact to
multiple entities (client, server, worker, database). See points above.


Changelog

In version v0.73.55 we accidentally fixed one half of a bug
that previously would not add dependency objects to @build
methods of classes.

Fixing the other half of the bug would potentially cause behavior
changes in image.imports, so this fix simply re-breaks deps generation
inside of containers to not cause errors
@freider freider requested a review from mwaskom February 21, 2025 20:47
@aksh-at
Copy link
Contributor

aksh-at commented Feb 22, 2025

Would be nice to add a test for this too, but merging this to fix for now.

@aksh-at aksh-at merged commit f9b168b into main Feb 22, 2025
24 checks passed
@aksh-at aksh-at deleted the freider/unfix-build-method-deps branch February 22, 2025 23:49
@freider
Copy link
Contributor Author

freider commented Feb 23, 2025

Yeah I was thinking of adding a test, but also feels a bit wrong to add a test for incorrect behavior.
I'll make a follow up PR on Monday with an integration test, since this has to do with the surface between worker and client

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.

2 participants