add proxy handler for PUT request #543
Conversation
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
gateway-ha/src/main/java/io/trino/gateway/proxyserver/ProxyRequestHandler.java
Outdated
Show resolved
Hide resolved
gateway-ha/src/main/java/io/trino/gateway/proxyserver/RouteToBackendResource.java
Outdated
Show resolved
Hide resolved
|
@siminyou please add a test |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
added test: |
gateway-ha/src/test/java/io/trino/gateway/ha/TestPutRequestHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Please squash commits into one and capitalize the commit message. https://trino.io/development/process.html#pull-request-and-commit-guidelines-
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package io.trino.gateway.ha; |
There was a problem hiding this comment.
Move to io.trino.gateway.proxyserver.
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| @TestInstance(Lifecycle.PER_CLASS) | ||
| public class TestPutRequestHandler |
There was a problem hiding this comment.
Remove public and add final. https://trino.io/docs/current/develop/tests.html
There was a problem hiding this comment.
Also, consider renaming to TestProxyRequestHandler. A test class per HTTP method is overkill.
| public class TestPutRequestHandler | ||
| { | ||
| private final OkHttpClient httpClient = new OkHttpClient(); | ||
| private MockWebServer mockTrinoServer = new MockWebServer(); |
| throws Exception | ||
| { | ||
| RequestBody requestBody = | ||
| RequestBody.create(MediaType.parse("application/json; charset=utf-8"), "SELECT 1"); |
| .build(); | ||
|
|
||
| try (Response response = httpClient.newCall(putRequest).execute()) { | ||
| assertThat(response.body().string()).isEqualTo(OK); |
| public void cleanup() | ||
| throws Exception | ||
| { | ||
| if (mockTrinoServer != null) { |
There was a problem hiding this comment.
I don't think mockTrinoServer is nullable.
| } | ||
|
|
||
| @Test | ||
| public void testPutRequestHandler() |
There was a problem hiding this comment.
Make test method package-private. https://trino.io/docs/current/develop/tests.html
Same for setup and cleanup methods.
Description
add handler for PUT request, which will be used in some trino distributions such as SEP. For instance, a PUT request is made to
ui/api/insights/openWorksheetsto save worksheetsAdditional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: