-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[vcl] don't manually interfere with compression #38211
[vcl] don't manually interfere with compression #38211
Conversation
Hi @gquintard. Thank you for your contribution! Add the comment under your pull request to deploy test or vanilla Magento instance:
❗ Automated tests can be triggered manually with an appropriate comment:
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
This was also proposed in #36796, but since it lacked feedback from Adobe people it got closed. Maybe taking things in smaller steps will have a bigger chance of being accepted (but knowing Adobe, it will still take months if not years 🙁 ) |
@magento run all tests |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues. |
@magento create issue |
@magento run Functional Tests B2B |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues. |
Hello @gquintard, Thanks for the contribution! We need a testing scenario to check this PR manually. Please provide us with the same. Meanwhile we are moving this PR Thanks |
well, that's unexpected: the "extra characters at the end of h command" error message makes no sense to me at the moment but I expect the (I've also quoted the path to avoid more issue, and it could solve the problem entirely, I'm unsure) |
@gquintard Thanks for the quick workaround! We are using Mac OS and set up the instance in our local machine. After taking the latest pull, the test is still failing. Please refer to the updated log below: We have just replaced our folder path with After digging into the compression.vtc file, I found that we need to pass an empty quote after the Please let us know if we missed anything. Thanks |
Varnish already has [default compression handling](https://varnish-cache.org/docs/trunk/users-guide/compression.html#default-behaviour). This code only kicks into gear in case of a `miss` or a `pass`, and in those cases, the backend should be responsible for these.
be8cd85
to
1e7a58f
Compare
@engcom-Hotel , thank you for your patience. I was able to get access to a mac thanks to the awesome @briiians, and it looks like on it, So, normally, this should be fixed, please have another look |
@magento run all tests |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues. |
@magento run Functional Tests B2B |
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues. |
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.
The failed Functional Tests B2B
test seems flaky to me.
Hence approving this PR.
✔️ QA Passed Varnish already has default compression handling. This code only kicks into gear in case of a miss or a pass, and in those cases, the backend should be responsible for these. Preconditions: Varnish should be enabled as Manual testing scenario: Run the below command to check the test result: Actual Result: ✔️ This PR is just an improvement. Tested all the manual scenarios, no impact on regression testing. |
795942b
into
magento:2.4-develop
Description (*)
Varnish already has default compression handling. This code only kicks into gear in case of a
miss
or apass
, and in those cases, the backend should be responsible for these.Manual testing scenarios (*)
automated testing should do the job
Contribution checklist (*)
Resolved issues: