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

[KYUUBI #3951][FOLLOWUP] Audit the rest request params #4389

Closed
wants to merge 3 commits into from

Conversation

turboFei
Copy link
Member

@turboFei turboFei commented Feb 21, 2023

Why are the changes needed?

Before:

user=anonymous(auth:BASIC) ip=127.0.0.1 proxyIp=null    method=GET      uri=/api/v1/operations/5e286c5d-2880-443f-a4e8-633964dcd699/rowset      protocol=HTTP/1.1       status=200

After:

user=anonymous(auth:BASIC) ip=127.0.0.1 proxyIp=null    method=GET      uri=/api/v1/operations/5e286c5d-2880-443f-a4e8-633964dcd699/rowset      params=maxrows=2&fetchorientation=FETCH_NEXT       protocol=HTTP/1.1       status=200
params=maxrows=2&fetchorientation=FETCH_NEXT

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

@turboFei
Copy link
Member Author

turboFei commented Feb 21, 2023

cc @lightning-L

with this pr, we can know the current maxRows specified by customers, and notify them to make change.

and set up the server side limitation (#4388 )

@turboFei turboFei closed this in d288a2b Feb 21, 2023
turboFei added a commit that referenced this pull request Feb 21, 2023
### _Why are the changes needed?_

Before:
```
user=anonymous(auth:BASIC) ip=127.0.0.1 proxyIp=null    method=GET      uri=/api/v1/operations/5e286c5d-2880-443f-a4e8-633964dcd699/rowset      protocol=HTTP/1.1       status=200
```

After:

```
user=anonymous(auth:BASIC) ip=127.0.0.1 proxyIp=null    method=GET      uri=/api/v1/operations/5e286c5d-2880-443f-a4e8-633964dcd699/rowset      params=maxrows=2&fetchorientation=FETCH_NEXT       protocol=HTTP/1.1       status=200
```

```
params=maxrows=2&fetchorientation=FETCH_NEXT
```
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [ ] Add screenshots for manual tests if appropriate

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4389 from turboFei/rest_params.

Closes #3951

6ffc1ad [fwang12] comments
61e12b1 [fwang12] nit
0632860 [fwang12] Audit the request params

Authored-by: fwang12 <fwang12@ebay.com>
Signed-off-by: fwang12 <fwang12@ebay.com>
(cherry picked from commit d288a2b)
Signed-off-by: fwang12 <fwang12@ebay.com>
@turboFei turboFei deleted the rest_params branch February 21, 2023 05:00
@turboFei
Copy link
Member Author

thanks, merged to master and branch-1.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants