-
Notifications
You must be signed in to change notification settings - Fork 49
Use redirect property on HTTP and OpenAPI calls #1013
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
Conversation
...ttp/src/main/java/io/serverlessworkflow/impl/executors/http/AbstractHttpExecutorBuilder.java
Outdated
Show resolved
Hide resolved
...ttp/src/main/java/io/serverlessworkflow/impl/executors/http/AbstractHttpExecutorBuilder.java
Outdated
Show resolved
Hide resolved
...ttp/src/main/java/io/serverlessworkflow/impl/executors/http/RedirectValidationException.java
Outdated
Show resolved
Hide resolved
...ttp/src/main/java/io/serverlessworkflow/impl/executors/http/AbstractHttpExecutorBuilder.java
Outdated
Show resolved
Hide resolved
...ttp/src/main/java/io/serverlessworkflow/impl/executors/http/AbstractHttpExecutorBuilder.java
Show resolved
Hide resolved
|
|
||
| if (!redirect) { | ||
| // disable automatic redirects handling from Jersey client | ||
| request.property("jersey.config.client.followRedirects", false); |
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.
We can't tie our implementation to this client. Dowstream client code can override the HTTP client factory, hence it won't be Jersey.
We could have a supplier here where the downstream client can be called to react to this property instead.
By default, we set the jersey property or let the supplier from the underlying client do so.
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.
We can't tie our implementation to this client. Dowstream client code can override the HTTP client factory, hence it won't be Jersey.
Yeah, I forget this spec/impl detail.
We could have a supplier here where the downstream client can be called to react to this property instead.
Do you mean a new method WorkflowApplication.Builder method or a ServiceProvider?
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.
We cannot have the client factory in WorkflowApplication.Builder, we need to specify it as additional object. This implies heavy changes here and will affect current users. So lets keep it for a different PR (that will also deal with timeouts), in this one you do not need to do anything.
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.
Ok, I will remove the jersey config.
|
@mcruzdev |
impl/http/src/main/java/io/serverlessworkflow/impl/executors/http/AbstractRequestSupplier.java
Outdated
Show resolved
Hide resolved
impl/http/src/main/java/io/serverlessworkflow/impl/executors/http/AbstractRequestSupplier.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
fjtirado
left a comment
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.
Thanks @mcruzdev
Changes
RuntimeExceptionthe theserverlessworkflow-impl-httpmodule calledRedirectValidationException.AbstractHttpExecutorBuilderresponsible for applying redirect validation rules for the HTTP response. When the validation fails, it pass the response status code to theWorkflowErrorinstance.TODO
redirectbehavior (when the upstream server returns a 3XX status code)Closes #959