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.

Sign up for GitHub

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

Jump to bottom

Discard http v2 #142

Merged
merged 16 commits into from
Oct 28, 2021
Merged

Discard http v2 #142

merged 16 commits into from
Oct 28, 2021

Conversation

ligurio
Copy link
Member

@ligurio ligurio commented Oct 22, 2021 •
edited
Loading

Fixes #134

@ligurio ligurio force-pushed the ligurio/gh-134-discard-v2 branch 25 times, most recently from 073ce2f to 0c5a706 Compare October 27, 2021 08:55
@ligurio ligurio force-pushed the ligurio/gh-134-discard-v2 branch 4 times, most recently from 7c53996 to d1d0198 Compare October 27, 2021 09:59
@ligurio
Copy link
Member Author

ligurio commented Oct 28, 2021 •
edited
Loading

I think you should extend description of this PR since it not only deprecates http v2, but also introduces GitHub Actions CI instead of Travis.

True. To be honestly, all commits with tests and Github Actions workflows and luacheck support were added for two reasons:

  • it is based on reverted commits and I don't want to lost work that was already done
  • testing, CI and luacheck can speed up a review and make life of reviewer a bit easy

@ligurio ligurio force-pushed the ligurio/gh-134-discard-v2 branch 2 times, most recently from 5766906 to 166ffad Compare October 28, 2021 11:20
@ligurio
Revert changes related to http v2
62d2349 
Patch reverts all changes made since commit 'add travis builds for tags'
(da1407c). All these changes are
related to http v2. Reasons are described in issue #134.

No test regressions introduced in comparison to http v1.
How to check:

cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo
make
make check

Part of #134
@ligurio
Add released versions and changes to changelog
b0c2aed 
Changes for versions 2.x.x were described in previous commits that later
were reverted in [1], this commit these changes back. Changes for
versions 1.x.x were not described in changelog and there is a ticket to
fix it [2].

1. #134
2. #143

Part of #134
@ligurio
ci: replace Travis CI with Github Actions
893aa7f 
Part of #134
@ligurio
ci: add workflow that publish rockspec
d943a5a 
Patch adds workflow for publishing rockspec in Github Actions. Initially
similar steps were added for Travis CI in commit 'Add rock publishing'
(3728483) but later reverted in scope
of issue with discard v2.

Follows up #109
Part of #134
@ligurio
test: rewrite TAP tests with luatest
7e429b7 
Patch adds a tests ported from TAP to luatest format that were
intitially implemented in commits 'Port from tap to luatest'
(549058d) and 'Rewrite tap tests to
luatest' (54d0fca) and later reverted in scope of issue with discard v2.

Follows up #90
Part of #134
@ligurio
Add editorconfig to configure indentation
daf50d7 
Patch adds a commit 'Add editorconfig to configure indents'
(886976a) that was implemented and the
reverted in scope of issue with discard v2.

Follows up #90
Part of #134
@ligurio
cmake: create separate target for running tests
32a11e0 
Run tests in CMake by a separate target ('make check') and not as a
CMake test.

Part of #134
@ligurio
cmake: fix FindTarantool.cmake module
c97c818 
Patch adds a commit 'Fix FindTarantool.cmake script'
(c6bd906) that was implemented in http
v2 and later reverted in scope of issue with discard v2.

Follows up #90
Part of #134
@ligurio
cmake: fix indentation
e4ee990
@ligurio
Add luacheck integration
cd1e7a5 
Patch reimplements commit 'Run luacheck on CI, fix warnings'
(b660d8e) added for http v2 and later
reverted in scope of issue with discard v2. File http/mime_types.lua
contains duplicate items in a table, I don't know what items can be
removed so I just ignored a file in a luacheck configuration file.

Follows up #95
Part of #134
@ligurio
http: replace io with fio module
7a803c7 
In tarantool the standart built-in module for working with files is "fio".
So we should use it instead of "io".

Patch adds a commit 'Get rid of io module'
(e0d8f82) that was implemented for http
v2 and later reverted in scope of issue with discard v2.

Follows up #90
Part of #134
@ligurio
http: fix SEGV_MAPERR when box httpd html escaping
dead340 
Patch adds a bug fix 'Check ptr to string before use it'
(05ad424) and test 'add a test'
(16c761a) for issue #51 that were
merged for http v2 and later reverted in scope of issue to discard v2.

Follows up #51
Part of #134
@ligurio
Update gitignore
7d566e2
@ligurio
Add option to get cookie without escaping
a56022e 
Method req:cookie() implicitly unescapes cookie values. Commit adds
ability to get cookie without unescaping:

req:cookie('name', {
    raw = true
})

This change was added as a part of http v2 support in commit 'Added
ability to set and get cookie without escaping'
(42e3002) and later reverted in scope
of ticket with discard v2.

Follows up #126
Part of #134
@ligurio
Add option to set cookie without escaping and change escaping algorithm
a46589b 
Method resp:setcookie() implicitly escapes cookie values. Commit adds
ability to set cookie without any escaping with option 'raw':

resp:setcookie('name', 'value', {
    raw = true
})`

Also added escaping for cookie path, and changed escaping algorithm
according to RFC 6265 "HTTP State Management Mechanism", see [1].

This change was added as a part of http v2 support in commit 'Added
ability to set and get cookie without escaping'
(42e3002) and later reverted in scope
of ticket with discard v2.

1. https://tools.ietf.org/html/rfc6265

Follows up #126
Part of #134
@ligurio
Add notes regarding to http v2 to the README
0a15a31 
Fixes #134
@ligurio ligurio merged commit 606fa09 into master Oct 28, 2021
ylobankov added a commit to tarantool/metrics that referenced this pull request Oct 29, 2021
@ylobankov
ci: fix http dependency in tests
aaf5867 
At this moment http dependency in tests is broken since we try to
install http 2.1.0 version. But http v2 is gone (tarantool/http#142)
and lua rock 2.1.0  is not available anymore. That is the reason of
test failures with the following error:

    http not found for Lua 5.1.
    Error: No results matching query were found for Lua 5.1.
    Checking if available for other Lua versions...
    make: *** [Makefile:10: .rocks] Error 1
    Checking for Lua 5.2...
    Checking for Lua 5.3...
    Checking for Lua 5.4...
    Error: Process completed with exit code 2.

So this patch switches use of http v2 to v1 that is the recommended
version for using.
ylobankov added a commit to tarantool/metrics that referenced this pull request Oct 29, 2021
@ylobankov
ci: fix http dependency in tests
7236677 
At this moment http dependency in tests is broken since we try to
install http 2.1.0 version. But http v2 is gone (tarantool/http#142)
and lua rock 2.1.0  is not available anymore. That is the reason for
test failures with the following error:

    http not found for Lua 5.1.
    Error: No results matching query were found for Lua 5.1.
    Checking if available for other Lua versions...
    make: *** [Makefile:10: .rocks] Error 1
    Checking for Lua 5.2...
    Checking for Lua 5.3...
    Checking for Lua 5.4...
    Error: Process completed with exit code 2.

So this patch switches the use of http v2 to v1 that is the recommended
version for using.
@ylobankov ylobankov mentioned this pull request Oct 29, 2021
Merged
ylobankov added a commit to tarantool/metrics that referenced this pull request Oct 29, 2021
@ylobankov
ci: fix http dependency in tests
2d2d86b 
At this moment http dependency in tests is broken since we are trying
to install the `http 2.1.0` lua rock. Now this rock is not available
anymore due to tarantool/http#142 and that is the reason for test
failures with the following error:

    http not found for Lua 5.1.
    Error: No results matching query were found for Lua 5.1.
    Checking if available for other Lua versions...
    make: *** [Makefile:10: .rocks] Error 1
    Checking for Lua 5.2...
    Checking for Lua 5.3...
    Checking for Lua 5.4...
    Error: Process completed with exit code 2.

Now http v2 is available under the `http-v2-legacy` lua rock and this
patch switches the use of `http 2.1.0` to `http-v2-legacy 2.1.0`.
yngvar-antonsson pushed a commit to tarantool/metrics that referenced this pull request Oct 29, 2021
@ylobankov
ci: fix http dependency in tests (#316)
da40cd4 
At this moment http dependency in tests is broken since we are trying
to install the `http 2.1.0` lua rock. Now this rock is not available
anymore due to tarantool/http#142 and that is the reason for test
failures with the following error:

    http not found for Lua 5.1.
    Error: No results matching query were found for Lua 5.1.
    Checking if available for other Lua versions...
    make: *** [Makefile:10: .rocks] Error 1
    Checking for Lua 5.2...
    Checking for Lua 5.3...
    Checking for Lua 5.4...
    Error: Process completed with exit code 2.

Now http v2 is available under the `http-v2-legacy` lua rock and this
patch switches the use of `http 2.1.0` to `http-v2-legacy 2.1.0`.
@Totktonada Totktonada deleted the ligurio/gh-134-discard-v2 branch November 8, 2021 05:01
@DifferentialOrange DifferentialOrange mentioned this pull request Dec 27, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DifferentialOrange DifferentialOrange DifferentialOrange approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Discard v2
2 participants
@ligurio @DifferentialOrange