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

SmallRye GraphQL Client extension + 1.2.1 upgrade #17350

Merged
merged 2 commits into from
May 20, 2021

Conversation

jmartisk
Copy link
Contributor

Fixes #16792

Subscription support and a user guide coming up next

@quarkus-bot quarkus-bot bot added area/core area/dependencies Pull requests that update a dependency file area/graphql area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/smallrye labels May 19, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented May 19, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 03cf425

Status Name Step Test failures Logs Raw logs
Initial JDK 11 Build Verify extension dependencies ⚠️ Check → Logs Raw logs

@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation labels May 19, 2021
@gsmet
Copy link
Member

gsmet commented May 19, 2021

That's very nice. I would be curious if we could get rid of all the custom stuff we have in https://github.com/quarkusio/status.quarkus.io/.

One of my questions is if it's possible to express queries as strings. We are using Qute templates in status.quarkus.io (see https://github.com/quarkusio/status.quarkus.io/blob/main/src/main/resources/templates/GitHubService/findIssuesByIds.graphql) and that's definitely easier to express/test than writing some DSL calls.

@jmartisk
Copy link
Contributor Author

Using string queries isn't supported by the dynamic client right now, but it won't be a problem to add that. (for the typesafe client, this obviously doesn't make sense)

@gsmet
Copy link
Member

gsmet commented May 19, 2021

@jmartisk I think it would be a useful experiment to try to convert status.quarkus.io to use this. And supporting String queries could be a very useful addition for that.

@jmartisk
Copy link
Contributor Author

@gsmet Definitely. I filed smallrye/smallrye-graphql#806

@quarkus-bot
Copy link

quarkus-bot bot commented May 19, 2021

Failing Jobs - Building c6a1798

Status Name Step Test failures Logs Raw logs
Maven Tests - JDK 11 Build Test failures Logs Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ Maven Tests - JDK 11 #

📦 integration-tests/maven

io.quarkus.maven.it.DevMojoIT.testThatNewResourcesAreServed line 841 - More details - Source on GitHub

@gsmet
Copy link
Member

gsmet commented May 19, 2021

@jmartisk in the doc, it would be useful to explain how to push an authorization header: https://github.com/quarkusio/status.quarkus.io/blob/main/src/main/java/io/quarkus/status/graphql/GraphQLClient.java#L14

@jmartisk
Copy link
Contributor Author

Sure. I'm planning to start writing the user guide today and see if I can get it done before going on PTO

@jmartisk
Copy link
Contributor Author

Oh, and a quickstart probably.
Anyway that will come in separate PRs.
I guess this is ready to merge, if no one has concerns. There's one CI failure that looks unrelated

@phillip-kruger
Copy link
Member

@gsmet - It would be good to get this in before the next Alpha (or Beta) so we can start playing :)

@gsmet gsmet merged commit 9838801 into quarkusio:main May 20, 2021
@quarkus-bot quarkus-bot bot added this to the 2.0 - main milestone May 20, 2021
@jmartisk jmartisk deleted the clients branch May 20, 2021 09:47
@t1
Copy link

t1 commented May 21, 2021

Great to see this coming! Any chance that it will be backported to pre-2.0?

And I had assumed I could just @Inject an instance of a @TypesafeGrapQLClientApi, but it throws a java.lang.NoSuchFieldError: io/quarkus/deployment/Feature.SMALLRYE_GRAPHQL_CLIENT. I didn't see any tests for this either. Should I open an issue for this?

@t1
Copy link

t1 commented May 21, 2021

sorry, my bad: I mixed versions, which is obviously a bad idea. Still there is no test for injecting typesafe client apis, only programmatic lookups.

@jmartisk
Copy link
Contributor Author

@t1 I'll add such test

@jmartisk
Copy link
Contributor Author

I don't think backporting is being planned, there's also the problem that Quarkus 1 contains SR GraphQL 1.0.x, which only contains the typesafe client, so it would have to be only a partial backport. And SR GraphQL 1.2.x depends on Vert.x 4 so it can't go into Quarkus 1.x. It would be quite complicated.

@jmartisk
Copy link
Contributor Author

@t1 btw here's the promised test #17401

@t1
Copy link

t1 commented May 21, 2021

That was fast! Thanks. Also for the explanation about not backporting. I'll have to do my demo talk with the Alpha-Releases then, I guess.

@phillip-kruger
Copy link
Member

@t1 when is your demo ?

@t1
Copy link

t1 commented May 21, 2021

@t1 when is your demo ?

On June 9th.. maybe I'll manage to get my GraphQL Federation Quarkus Extension running by then (this is all very new to me). That would be uber cool. Otherwise I'll have to switch back to WildFly.

@phillip-kruger
Copy link
Member

@t1 - By then Quarkus 2 should be released (see https://github.com/quarkusio/quarkus/wiki/Release-Planning), also, subscription is also in, have a look :)

@t1
Copy link

t1 commented May 21, 2021

Cool... it says 'Full Platform release (and official announcement) on June 9th' ... that would be exactly the day... coincidence or destiny? ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/graphql area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/smallrye release/noteworthy-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SmallRye GraphQL Client extension
4 participants