Skip to content

fix: correct the compressor choose#801

Merged
hezhangjian merged 2 commits into
openGemini:mainfrom
xkx9431:xkx/fix_content
Nov 21, 2024
Merged

fix: correct the compressor choose#801
hezhangjian merged 2 commits into
openGemini:mainfrom
xkx9431:xkx/fix_content

Conversation

@xkx9431

@xkx9431 xkx9431 commented Nov 21, 2024

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: fix #791

What is changed and how it works?

Prior to the PR, there were two issues:

  1. We should use the HTTP header Accept-Encoding instead of content-encoding to determine the compressor.

Todo

  • we also need to consider the content negotiation if http headers contains a list of accept encoding suits.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Unit Test
  • Integration Test
  • Test cases to be added
  • No code

validation

image

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Signed-off-by: xkx <triumph_9431@qq.com>
@xkx9431

xkx9431 commented Nov 21, 2024

Copy link
Copy Markdown
Contributor Author

@vicky-run @fx408 Please help review and merge this, I changed this from copilot code gen wrong.

@hezhangjian hezhangjian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some unit tests?

Signed-off-by: xkx <triumph_9431@qq.com>
@xkx9431 xkx9431 requested a review from hezhangjian November 21, 2024 02:07

@hezhangjian hezhangjian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hezhangjian hezhangjian merged commit c12bda1 into openGemini:main Nov 21, 2024
xuthus5 pushed a commit that referenced this pull request Mar 20, 2026
<!-- Thank you for contributing to openGemini! -->

<!-- Mark the checkbox [X] or [x] if you agree with the item. -->

### What problem does this PR solve?
<!--

Please create an issue first to describe the problem.

There MUST be one line starting with "Issue Number:  " and
linking the relevant issues via the "close","fix", "resolve" or "ref".
-->

Issue Number: fix #791 

### What is changed and how it works?

Prior to the PR, there were two issues:
1. We should use the HTTP header **Accept-Encoding** instead of content-encoding to determine the compressor.

## Todo
  - [ ]  we also need to consider the content negotiation if http headers contains a list of accept encoding suits.


### How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

- [x] Unit Test 
- [x] Integration Test 
- [ ] Test cases to be added
- [ ] No code

## validation
![image](https://github.com/user-attachments/assets/ee189ee7-5776-4f17-b4c2-9a6882f57e9f)


# Checklist:

- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules

Signed-off-by: xkx <triumph_9431@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants