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

buildengine can reach a deadend where build errors can't be easily fixed by user #1211

Closed
matt2e opened this issue Apr 9, 2024 · 1 comment · Fixed by #1217
Closed

buildengine can reach a deadend where build errors can't be easily fixed by user #1211

matt2e opened this issue Apr 9, 2024 · 1 comment · Fixed by #1217
Labels
triage Issue needs triaging

Comments

@matt2e
Copy link
Collaborator

matt2e commented Apr 9, 2024 •
edited
Loading

Steps to repro:

  • start ftl dev with 2 modules, lets call them moduleA and moduleB
  • in moduleA, create a verb and call it VerbA
  • in moduleB, make a call to to the verb moduleA.VerbA
    • ftl autodetects the change and deploys both modules
  • update moduleB to call modulaA.VerbB (even though it doesnt exist)
    • ftl autodetects the change and tries to build moduleB but fails. This is expected
  • update moduleA's verb to be called VerbB instead of VerbA
    • ftl autodetects the change and tries to build moduleA and succeeds.
    • ftl then tries to deploy moduleA and fails:

error:controller0: Invalid module schema: invalid schema: .../moduleB/moduleB.go:29:16-16: unknown reference "moduleA.verbA"

The user can't really do much to untie this knot. Editing moduleB in small ways keeps failing with unknown reference for verbB. updating modulaA fails because a deployed module refers to verbA

  • terminating ftl dev and rerunning it fixes the issue
@github-actions github-actions bot added the triage Issue needs triaging label Apr 9, 2024
@alecthomas alecthomas mentioned this issue Apr 9, 2024
Open
@wesbillman
Copy link
Collaborator

Great repro steps @matt2e!!!

Took a quick look at this one and we're hitting this validation error, which makes sense since the whole schema won't validate with calls to unknown types.
https://github.com/TBD54566975/ftl/blob/main/backend/schema/validate.go#L126-L128

Removing that check makes everything deploy correct, but leaves us open to deploying invalid schema so we'll probably need a more robust solution

worstell added a commit that referenced this issue Apr 9, 2024
@worstell
fix: validate deploying module rather than entire schema
7708773 
with this change, when deploying a module, we will only validate the deploying module's schema (but do so within the context of the greater schema).
this change prevents issues where a schema error outside of the deploying module prevents the deploy from succeeding—causing issues like #1211.
now in combination with the buildengine logic, a change in the upstream module that breaks a downstream module will not prevent the change from deploying (if the deploying module still has a valid standalone schema). the deploy will succeed and then the subsequent deploy triggered for the downstream module will fail if the schema change from its dependency is breaking.

fixes #1211
@worstell worstell mentioned this issue Apr 9, 2024
Merged
worstell added a commit that referenced this issue Apr 9, 2024
@worstell
fix: validate deploying module rather than entire schema
2ece28b 
with this change, when deploying a module, we will only validate the deploying module's schema (but do so within the context of the greater schema).
this change prevents issues where a schema error outside of the deploying module prevents the deploy from succeeding—causing issues like #1211.
now in combination with the buildengine logic, a change in the upstream module that breaks a downstream module will not prevent the change from deploying (if the deploying module still has a valid standalone schema). the deploy will succeed and then the subsequent deploy triggered for the downstream module will fail if the schema change from its dependency is breaking.

fixes #1211
worstell added a commit that referenced this issue Apr 9, 2024
@worstell
fix: validate deploying module rather than entire schema
14f7140 
with this change, when deploying a module, we will only validate the deploying module's schema (but do so within the context of the greater schema).
this change prevents issues where a schema error outside of the deploying module prevents the deploy from succeeding—causing issues like #1211.
now in combination with the buildengine logic, a change in the upstream module that breaks a downstream module will not prevent the change from deploying (if the deploying module still has a valid standalone schema). the deploy will succeed and then the subsequent deploy triggered for the downstream module will fail if the schema change from its dependency is breaking.

fixes #1211
worstell added a commit that referenced this issue Apr 9, 2024
@worstell
fix: validate deploying module rather than entire schema (#1217)
1896cca 
when deploying a module, we will only validate the deploying module's
schema (but do so within the context of the greater schema).

this change prevents issues where a schema error outside of the
deploying module prevents the deploy from succeeding—causing issues like
#1211. now in combination with the buildengine logic, a change in the
upstream module that breaks a downstream module will not prevent the
upstream module from deploying (if the module still has a valid
standalone schema). the deploy will succeed and then the subsequent
deploy triggered for the downstream module will fail if the schema
change from its dependency breaks it.

fixes #1211
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
triage Issue needs triaging
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: validate deploying module rather than entire schema block/ftl
2 participants
@wesbillman @matt2e