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

fix: propagate graphql client to child clients #6851

Merged
merged 1 commit into from Mar 8, 2024

Conversation

jedevc
Copy link
Member

@jedevc jedevc commented Mar 7, 2024

Should fix #6850 (cc @morlay)

This regressed in #6716, and meant that child clients were invalid, and couldn't be called with Do(), as well as causing GraphQLClient() to return an invalid nil value.

This only previously worked because the GraphQL client and the dagger client shared the same name client. When this property was removed, the propagation stopped working. Now, we make this special case explicit.


@morlay if you have a more specific stack trace from the panic, or can somehow confirm that this fixes your issue that would be super helpful.

This regressed in 62ced56, and meant
that child clients were invalid, and couldn't be called with `Do()`, as
well as causing `GraphQLClient()` to return an invalid nil value.

This only previously worked because the GraphQL client and the dagger
client shared the same name `client`. When this property was removed,
the propagation stopped working. Now, we make this special case
explicit.

Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc added this to the v0.10.2 milestone Mar 7, 2024
@jedevc jedevc requested a review from vito March 7, 2024 14:45
@morlay
Copy link
Contributor

morlay commented Mar 7, 2024

https://github.com/octohelm/wagon/blob/main/pkg/engine/daggerutil/querybuilder.go#L9

panic in dagger.Client.Do(), .client is nil

I found only happen on the Client returned by Pipeline().

@jedevc jedevc merged commit c1b0b5b into dagger:main Mar 8, 2024
43 checks passed
@jedevc jedevc deleted the fix-client-pipeline-nil branch March 8, 2024 09:38
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.

🐞 v0.10.1 Client panic when with Pipeline
3 participants