-
Notifications
You must be signed in to change notification settings - Fork 3
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
Retain deployId
query param from PUT
request
#86
Conversation
fe03e27
to
571c660
Compare
mockWebServer.shutdown(); | ||
} | ||
|
||
private void assertThatPayloadDoesNotContainDeployId(RecordedRequest recordedRequest) throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/inngest/inngest/blob/main/docs/SDK_SPEC.md#432-syncing
If a deployId query string parameter is set on the sync request received by the SDK, the SDK MUST set the same key and value on the query string of the POST request to Inngest and MUST NOT include the deployId in the URL included in the Sync’s payload. The deployId is used to attribute various parts of a sync’s handshake with each other.
I could add a comment linking the spec doc but i'm not sure if it's a good idea.
1822df5
to
7da679e
Compare
571c660
to
59a8147
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, especially the work to set up the test with the mock server
inngest-spring-boot-demo/src/test/java/com/inngest/springbootdemo/SyncRequestTest.java
Outdated
Show resolved
Hide resolved
@Import(SyncInngestConfiguration.class) | ||
@WebMvcTest(DemoController.class) | ||
@Nested | ||
@EnabledIfSystemProperty(named = "test-group", matches = "unit-test") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to ask this on another PR, but what exactly does this do? Run it only in make test
or make itest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It runs only on make test
, the property is set in:
systemProperty("test-group", "unit-test") |
I'm open to better ways to accomplish this though.
Add a few explaining comments.
Summary
Syncing from the cloud platform by giving it an ngrok URL used to fail before this change and should work as expected with this.
deployId
query parameter from thePUT
request to thePOST
Sync call.I followed this doc for mocking the HTTP client: https://github.com/square/okhttp/tree/master/mockwebserver
Checklist
Related