Default response content type using GraphQL spec #35769
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Default response content type header should conform to [GraphQL over HTTP spec](https://github.com/graphql/graphql-over-http/blob/main/spec/GraphQLOverHTTP.md#serialization-format).
|
@jmartisk can you have look ? |
This comment has been minimized.
This comment has been minimized.
jmartisk
requested changes
Sep 12, 2023
There was a problem hiding this comment.
Tests are failing - you need to also update https://github.com/quarkusio/quarkus/blob/3.4.0.CR1/extensions/smallrye-graphql/runtime/src/main/java/io/quarkus/smallrye/graphql/runtime/SmallRyeGraphQLExecutionHandler.java#L299 - please update it to accept both the new (fixed) content-type and the original one, for backward compatibility
|
@jmartisk this hasn't made any progress and I think it's really something that needs fixing. Could you have a look? Thanks. |
|
I've added a commit that hopefully completes the fix |
This comment has been minimized.
This comment has been minimized.
gsmet
requested changes
Oct 6, 2023
| @@ -297,7 +297,8 @@ private static String getCharset(String mimeType) { | |||
| private boolean isValidAcceptRequest(String mimeType) { | |||
| // At this point we only accept two | |||
There was a problem hiding this comment.
Change looks good to fix the tests but this comment looks outdated. I wonder if we should explain a bit more and put the two valid ones first and the last one with a comment // deprecated content-type supported for compatibility.
There was a problem hiding this comment.
Ah, that's a very good observation, thanks. Fixed
|
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. |
jmartisk
approved these changes
Oct 6, 2023
phillip-kruger
approved these changes
Oct 7, 2023
|
@gsmet can you please have another look? I would merge it but I can't as long as there's a -1 from you :) |
gsmet
approved these changes
Oct 11, 2023
benkard
pushed a commit
to benkard/mulkcms2
that referenced
this pull request
Nov 12, 2023
This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.217.0` -> `^0.219.0`](https://renovatebot.com/diffs/npm/flow-bin/0.217.2/0.219.0) | | [io.hypersistence:hypersistence-utils-hibernate-62](https://github.com/vladmihalcea/hypersistence-utils) | compile | minor | `3.5.3` -> `3.6.0` | | [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.4.2` -> `3.4.3` | | [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | patch | `3.4.2` -> `3.4.3` | --- ### Release Notes <details> <summary>flowtype/flow-bin</summary> ### [`v0.219.0`](flow/flow-bin@1268ec5...c184c5d) [Compare Source](flow/flow-bin@1268ec5...c184c5d) ### [`v0.218.1`](flow/flow-bin@12afce4...1268ec5) [Compare Source](flow/flow-bin@12afce4...1268ec5) ### [`v0.218.0`](flow/flow-bin@dc93913...12afce4) [Compare Source](flow/flow-bin@dc93913...12afce4) </details> <details> <summary>vladmihalcea/hypersistence-utils</summary> ### [`v3.6.0`](https://github.com/vladmihalcea/hypersistence-utils/blob/HEAD/changelog.txt#Version-360---October-12-2023) \================================================================================ Implement QueryStackTraceLogger using StackWalker [#​659](vladmihalcea/hypersistence-utils#659) Add JFR based query logger [#​658](vladmihalcea/hypersistence-utils#658) Adds support for using MonetaryAmount in [@​ElementCollection](https://github.com/ElementCollection) [#​652](vladmihalcea/hypersistence-utils#652) </details> <details> <summary>quarkusio/quarkus</summary> ### [`v3.4.3`](https://github.com/quarkusio/quarkus/releases/tag/3.4.3) [Compare Source](quarkusio/quarkus@3.4.2...3.4.3) ##### Complete changelog - [#​36408](quarkusio/quarkus#36408) - Ensure that SSE builder works in native - [#​36404](quarkusio/quarkus#36404) - Do not exclude properties from recording that are available in sources that should always be included - [#​36403](quarkusio/quarkus#36403) - IBM Db2 - Register resource bundle classes for reflection - [#​36402](quarkusio/quarkus#36402) - Native Picocli build breaks SSE client - [#​36399](quarkusio/quarkus#36399) - quarkus-jdbc-db2: resource bundle missing - [#​36377](quarkusio/quarkus#36377) - Allow `@ClientHeaderParam` to override User-Agent - [#​36371](quarkusio/quarkus#36371) - Fix issue in Java migration in dev-mode - [#​36351](quarkusio/quarkus#36351) - Properly handle invalid response body errors in Reactive REST Client - [#​36329](quarkusio/quarkus#36329) - Custom User-Agent header ignored - [#​36326](quarkusio/quarkus#36326) - Cannot load fixed or default YAML configuration when running native build - [#​36302](quarkusio/quarkus#36302) - Fix headers and preambles in all guides and reintroduce some keywords - [#​36299](quarkusio/quarkus#36299) - quarkus-flyway: Java-Migration does not get picked up on live reload - [#​36290](quarkusio/quarkus#36290) - Fixed URL for configuring JSON support - [#​36257](quarkusio/quarkus#36257) - Rest client call hangs when receiving an invalid chunked response and does not release resources (e.g. Bulkhead semaphore) - [#​36147](quarkusio/quarkus#36147) - Bump org.eclipse.parsson:parsson from 1.1.2 to 1.1.4 - [#​36096](quarkusio/quarkus#36096) - Build cache - Use notCacheableBecause instead of storeEnabled - [#​35929](quarkusio/quarkus#35929) - Do not store build cache for core extensions having config - [#​35927](quarkusio/quarkus#35927) - Build cache - Core extensions containing config shouldn't be cached - [#​35769](quarkusio/quarkus#35769) - Default response content type using GraphQL spec </details> <details> <summary>quarkusio/quarkus-platform</summary> ### [`v3.4.3`](quarkusio/quarkus-platform@3.4.2...3.4.3) [Compare Source](quarkusio/quarkus-platform@3.4.2...3.4.3) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So the default content type header was not recognized properly by GraphQL gateway. I tried to submit PR but they said it doesn't conform to the GraphQL spec.