[Tarantool-patches] [PATCH v1 0/4] Support Python 3 in Tarantool tests

[Tarantool-patches] [PATCH v1 0/4] Support Python 3 in Tarantool tests

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <estetus@gmail.com>

It's another part of fixes related to Python 3 support.  First one was here
[1]. I haven't create new issue because the task is the same - switch test
suite to Python 3, so two related commits in a patch series marked with "Part
of #5538" and no one commit closes any task as it is required by guidelines.

To test fixes one can use test-run's branch with Python 3 support [2].

1. https://lists.tarantool.org/pipermail/tarantool-patches/2020-December/021151.html
2. https://github.com/tarantool/test-run/pull/263

Issue: https://github.com/tarantool/tarantool/issues/5538 (closed)
Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/240927581

Sergey Bronnikov (4):
  test: fix app-tap/http_client.test.lua
  test: fix xlog-py/big_lsn.test.py
  test: enable SO_REUSEADDR on socket in httpd.py
  test: enable disabled testcases back

 test/app-tap/http_client.test.lua |  6 +-----
 test/app-tap/httpd.py             | 21 +++++++++++----------
 test/xlog-py/big_lsn.test.py      |  3 ++-
 3 files changed, 14 insertions(+), 16 deletions(-)

-- 
2.25.1

[Tarantool-patches] [PATCH v1 1/4] test: fix app-tap/http_client.test.lua

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <estetus@gmail.com>

Pass http body as byte string and define string literals correctly.

Part of #5538
---
 test/app-tap/httpd.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/test/app-tap/httpd.py b/test/app-tap/httpd.py
index a2dee1b83..62435e91a 100755
--- a/test/app-tap/httpd.py
+++ b/test/app-tap/httpd.py
@@ -7,24 +7,24 @@ from gevent import spawn, sleep, socket
 def absent():
     code = "500 Server Error"
     headers = [("Content-Type", "application/json")]
-    body = ["No such method"]
+    body = [b'No such method']
     return code, body, headers
 
 def hello():
     code = "200 OK"
-    body = ["hello world"]
+    body = [b'hello world']
     headers = [("Content-Type", "application/json")]
     return code, body, headers
 
 def hello1():
     code = "200 OK"
-    body = [b"abc"]
+    body = [b'abc']
     headers = [("Content-Type", "application/json")]
     return code, body, headers
 
 def headers():
     code = "200 OK"
-    body = [b"cookies"]
+    body = [b'cookies']
     headers = [("Content-Type", "application/json"),
                ("Content-Type", "application/yaml"),
                ("Set-Cookie", "likes=cheese; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Secure; HttpOnly"),
@@ -41,13 +41,13 @@ def headers():
 def long_query():
     sleep(0.005)
     code = "200 OK"
-    body = [b"abc"]
+    body = [b'abc']
     headers = [("Content-Type", "application/json")]
     return code, body, headers
 
 def redirect():
     code = "302 Found"
-    body = ["redirecting"]
+    body = [b'redirecting']
     headers = [("Location", "/")]
     return code, body, headers
 
@@ -63,7 +63,7 @@ paths = {
 def read_handle(env, response):
     code = "404 Not Found"
     headers = []
-    body = ["Not Found"]
+    body = [b'Not Found']
     if env["PATH_INFO"] in paths:
         code, body, headers = paths[env["PATH_INFO"]]()
     for key,value in iter(env.items()):
@@ -76,7 +76,7 @@ def post_handle(env, response):
     code = "200 OK"
     body = [env["wsgi.input"].read()]
     headers = []
-    for key,value in env.iteritems():
+    for key,value in iter(env.items()):
         if "HTTP_" in key:
             headers.append((key[5:].lower(), value))
     response(code, headers)
@@ -84,8 +84,8 @@ def post_handle(env, response):
 
 def other_handle(env, response, method, code):
     headers = [("Content-Type", "text/plain"), ("method", method)]
-    body = [method]
-    for key,value in env.iteritems():
+    body = [method.encode('utf-8')]
+    for key,value in iter(env.items()):
         if "HTTP_" in key:
             headers.append((key[5:].lower(), value))
     response(code, headers)
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v1 1/4] test: fix app-tap/http_client.test.lua

[Leonid Vasiliev via Tarantool-patches]

Hi! Thank you for the patch.

Generally LGTM. See some comments below:

On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <estetus@gmail.com>
> 
> Pass http body as byte string and define string literals correctly.
> 
> Part of #5538

AFAIU, if the task has been closed, new patches corresponding with the
task are marked as "Follows up".

> ---
>   test/app-tap/httpd.py | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/test/app-tap/httpd.py b/test/app-tap/httpd.py
> index a2dee1b83..62435e91a 100755
> --- a/test/app-tap/httpd.py
> +++ b/test/app-tap/httpd.py
> @@ -7,24 +7,24 @@ from gevent import spawn, sleep, socket
>   def absent():
>       code = "500 Server Error"
>       headers = [("Content-Type", "application/json")]
> -    body = ["No such method"]
> +    body = [b'No such method']
>       return code, body, headers
>   
>   def hello():
>       code = "200 OK"
> -    body = ["hello world"]
> +    body = [b'hello world']
>       headers = [("Content-Type", "application/json")]
>       return code, body, headers
>   
>   def hello1():
>       code = "200 OK"
> -    body = [b"abc"]
> +    body = [b'abc']
>       headers = [("Content-Type", "application/json")]
>       return code, body, headers
>   
>   def headers():
>       code = "200 OK"
> -    body = [b"cookies"]
> +    body = [b'cookies']
>       headers = [("Content-Type", "application/json"),
>                  ("Content-Type", "application/yaml"),
>                  ("Set-Cookie", "likes=cheese; Expires=Wed, 21 Oct 2015 07:28:00 GMT; Secure; HttpOnly"),
> @@ -41,13 +41,13 @@ def headers():
>   def long_query():
>       sleep(0.005)
>       code = "200 OK"
> -    body = [b"abc"]
> +    body = [b'abc']
>       headers = [("Content-Type", "application/json")]
>       return code, body, headers
>   
>   def redirect():
>       code = "302 Found"
> -    body = ["redirecting"]
> +    body = [b'redirecting']
>       headers = [("Location", "/")]
>       return code, body, headers
>   
> @@ -63,7 +63,7 @@ paths = {
>   def read_handle(env, response):
>       code = "404 Not Found"
>       headers = []
> -    body = ["Not Found"]
> +    body = [b'Not Found']
>       if env["PATH_INFO"] in paths:
>           code, body, headers = paths[env["PATH_INFO"]]()
>       for key,value in iter(env.items()):
> @@ -76,7 +76,7 @@ def post_handle(env, response):
>       code = "200 OK"
>       body = [env["wsgi.input"].read()]
>       headers = []
> -    for key,value in env.iteritems():
> +    for key,value in iter(env.items()):

This change is not about "Pass http body as byte string and define 
string literals correctly." Please edit the commit message or move this 
change to a separate commit.

>           if "HTTP_" in key:
>               headers.append((key[5:].lower(), value))
>       response(code, headers)
> @@ -84,8 +84,8 @@ def post_handle(env, response):
>   
>   def other_handle(env, response, method, code):
>       headers = [("Content-Type", "text/plain"), ("method", method)]
> -    body = [method]
> -    for key,value in env.iteritems():
> +    body = [method.encode('utf-8')]

> +    for key,value in iter(env.items()):

This change is not about "Pass http body as byte string and define 
string literals correctly." Please edit the commit message or move this 
change to a separate commit.

>           if "HTTP_" in key:
>               headers.append((key[5:].lower(), value))
>       response(code, headers)
> 

Re: [Tarantool-patches] [PATCH v1 1/4] test: fix app-tap/http_client.test.lua

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

On 14.01.2021 11:47, Leonid Vasiliev wrote:
> Hi! Thank you for the patch.
>
> Generally LGTM. See some comments below:
>
> On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
>> From: Sergey Bronnikov <estetus@gmail.com>
>>
>> Pass http body as byte string and define string literals correctly.
>>
>> Part of #5538
>
> AFAIU, if the task has been closed, new patches corresponding with the
> task are marked as "Follows up".
>
Updated commit messages.
>> ---
>>   test/app-tap/httpd.py | 20 ++++++++++----------
>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/test/app-tap/httpd.py b/test/app-tap/httpd.py
>> index a2dee1b83..62435e91a 100755
>> --- a/test/app-tap/httpd.py
>> +++ b/test/app-tap/httpd.py
>> @@ -7,24 +7,24 @@ from gevent import spawn, sleep, socket
>>   def absent():
>>       code = "500 Server Error"
>>       headers = [("Content-Type", "application/json")]
>> -    body = ["No such method"]
>> +    body = [b'No such method']
>>       return code, body, headers
>>     def hello():
>>       code = "200 OK"
>> -    body = ["hello world"]
>> +    body = [b'hello world']
>>       headers = [("Content-Type", "application/json")]
>>       return code, body, headers
>>     def hello1():
>>       code = "200 OK"
>> -    body = [b"abc"]
>> +    body = [b'abc']
>>       headers = [("Content-Type", "application/json")]
>>       return code, body, headers
>>     def headers():
>>       code = "200 OK"
>> -    body = [b"cookies"]
>> +    body = [b'cookies']
>>       headers = [("Content-Type", "application/json"),
>>                  ("Content-Type", "application/yaml"),
>>                  ("Set-Cookie", "likes=cheese; Expires=Wed, 21 Oct 
>> 2015 07:28:00 GMT; Secure; HttpOnly"),
>> @@ -41,13 +41,13 @@ def headers():
>>   def long_query():
>>       sleep(0.005)
>>       code = "200 OK"
>> -    body = [b"abc"]
>> +    body = [b'abc']
>>       headers = [("Content-Type", "application/json")]
>>       return code, body, headers
>>     def redirect():
>>       code = "302 Found"
>> -    body = ["redirecting"]
>> +    body = [b'redirecting']
>>       headers = [("Location", "/")]
>>       return code, body, headers
>>   @@ -63,7 +63,7 @@ paths = {
>>   def read_handle(env, response):
>>       code = "404 Not Found"
>>       headers = []
>> -    body = ["Not Found"]
>> +    body = [b'Not Found']
>>       if env["PATH_INFO"] in paths:
>>           code, body, headers = paths[env["PATH_INFO"]]()
>>       for key,value in iter(env.items()):
>> @@ -76,7 +76,7 @@ def post_handle(env, response):
>>       code = "200 OK"
>>       body = [env["wsgi.input"].read()]
>>       headers = []
>> -    for key,value in env.iteritems():
>> +    for key,value in iter(env.items()):
>
> This change is not about "Pass http body as byte string and define 
> string literals correctly." Please edit the commit message or move 
> this change to a separate commit.
>
commit 00c194f09b230cd01f38ed7e08e5fcc7e2d56928
Author: Sergey Bronnikov <sergeyb@tarantool.org>
Date:   Mon Dec 28 15:15:41 2020 +0300

     test: fix app-tap/http_client.test.lua

     Pass http body as byte string, define string literals correctly
     and make items() iterable.

     Follows up #5538

>>           if "HTTP_" in key:
>>               headers.append((key[5:].lower(), value))
>>       response(code, headers)
>> @@ -84,8 +84,8 @@ def post_handle(env, response):
>>     def other_handle(env, response, method, code):
>>       headers = [("Content-Type", "text/plain"), ("method", method)]
>> -    body = [method]
>> -    for key,value in env.iteritems():
>> +    body = [method.encode('utf-8')]
>
>> +    for key,value in iter(env.items()):
>
> This change is not about "Pass http body as byte string and define 
> string literals correctly." Please edit the commit message or move 
> this change to a separate commit.
>
>>           if "HTTP_" in key:
>>               headers.append((key[5:].lower(), value))
>>       response(code, headers)
>>

Re: [Tarantool-patches] [PATCH v1 1/4] test: fix app-tap/http_client.test.lua

[Alexander Turenko via Tarantool-patches]

> @@ -76,7 +76,7 @@ def post_handle(env, response):
>      code = "200 OK"
>      body = [env["wsgi.input"].read()]
>      headers = []
> -    for key,value in env.iteritems():
> +    for key,value in iter(env.items()):

AFAIR, <dict>.items() forms a list on Python 2, but creates an iterator
object on Python 3. Both are suitable for the for loop expression and
there is no much difference when the dictionary in not large.

iter() does not change anything here (however it is harmless).

.iteritems() was preferred by developers in Python 2 days, because it
creates an iterator, but most of times we know that a dictionary is not
large and we're okay with negligible perf. down on Python 2.

Re: [Tarantool-patches] [PATCH v1 1/4] test: fix app-tap/http_client.test.lua

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

On 14.01.2021 15:23, Alexander Turenko wrote:
>> @@ -76,7 +76,7 @@ def post_handle(env, response):
>>       code = "200 OK"
>>       body = [env["wsgi.input"].read()]
>>       headers = []
>> -    for key,value in env.iteritems():
>> +    for key,value in iter(env.items()):
> AFAIR, <dict>.items() forms a list on Python 2, but creates an iterator
> object on Python 3. Both are suitable for the for loop expression and
> there is no much difference when the dictionary in not large.

Generally it is so. Although PEP-0469 recommends to convert d.items() to 
iter(d.iteritems()) [1].

> iter() does not change anything here (however it is harmless).
>
> .iteritems() was preferred by developers in Python 2 days, because it
> creates an iterator, but most of times we know that a dictionary is not
> large and we're okay with negligible perf. down on Python 2.

I have reverted iter() related changes to reduce a patch. Everything 
works as is.

1. https://www.python.org/dev/peps/pep-0469/

Re: [Tarantool-patches] [PATCH v1 1/4] test: fix app-tap/http_client.test.lua

[Alexander Turenko via Tarantool-patches]

On Thu, Jan 14, 2021 at 03:52:06PM +0300, Sergey Bronnikov wrote:
> Thanks for review!
> 
> On 14.01.2021 15:23, Alexander Turenko wrote:
> > > @@ -76,7 +76,7 @@ def post_handle(env, response):
> > >       code = "200 OK"
> > >       body = [env["wsgi.input"].read()]
> > >       headers = []
> > > -    for key,value in env.iteritems():
> > > +    for key,value in iter(env.items()):
> > AFAIR, <dict>.items() forms a list on Python 2, but creates an iterator
> > object on Python 3. Both are suitable for the for loop expression and
> > there is no much difference when the dictionary in not large.
> 
> Generally it is so. Although PEP-0469 recommends to convert d.items() to
> iter(d.iteritems()) [1].

It is the most common way: so if one pass the iterator to some function,
which expects an iterator object (but not a list), it'll work.

However for most usages (such as in a loop expressions), .items() is
enough.

[Tarantool-patches] [PATCH v1 2/4] test: fix xlog-py/big_lsn.test.py

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <estetus@gmail.com>

Set correct encoding on opening file.

Part of #5538
---
 test/xlog-py/big_lsn.test.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/xlog-py/big_lsn.test.py b/test/xlog-py/big_lsn.test.py
index edc1e5620..73dcf7a48 100644
--- a/test/xlog-py/big_lsn.test.py
+++ b/test/xlog-py/big_lsn.test.py
@@ -1,4 +1,5 @@
 import os
+import codecs
 
 #
 # Check that Tarantool handles huge LSNs well (gh-4033).
@@ -17,7 +18,7 @@ new_lsn = 123456789123
 wal_dir = os.path.join(server.vardir, server.name)
 old_wal = os.path.join(wal_dir, "%020d.xlog" % old_lsn)
 new_wal = os.path.join(wal_dir, "%020d.xlog" % new_lsn)
-with open(old_wal, "r+") as f:
+with codecs.open(old_wal, "r+", encoding = "ISO-8859-1") as f:
     s = f.read()
     s = s.replace("VClock: {{1: {}}}".format(old_lsn),
                   "VClock: {{1: {}}}".format(new_lsn))
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v1 2/4] test: fix xlog-py/big_lsn.test.py

[Leonid Vasiliev via Tarantool-patches]

Hi! Thank you for the patch.
LGTM.

On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <estetus@gmail.com>
> 
> Set correct encoding on opening file.
> 
> Part of #5538

AFAIU, if the task has been closed, new patches corresponding with the
task are marked as "Follows up".

> ---
>   test/xlog-py/big_lsn.test.py | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/test/xlog-py/big_lsn.test.py b/test/xlog-py/big_lsn.test.py
> index edc1e5620..73dcf7a48 100644
> --- a/test/xlog-py/big_lsn.test.py
> +++ b/test/xlog-py/big_lsn.test.py
> @@ -1,4 +1,5 @@
>   import os
> +import codecs
>   
>   #
>   # Check that Tarantool handles huge LSNs well (gh-4033).
> @@ -17,7 +18,7 @@ new_lsn = 123456789123
>   wal_dir = os.path.join(server.vardir, server.name)
>   old_wal = os.path.join(wal_dir, "%020d.xlog" % old_lsn)
>   new_wal = os.path.join(wal_dir, "%020d.xlog" % new_lsn)
> -with open(old_wal, "r+") as f:
> +with codecs.open(old_wal, "r+", encoding = "ISO-8859-1") as f:
>       s = f.read()
>       s = s.replace("VClock: {{1: {}}}".format(old_lsn),
>                     "VClock: {{1: {}}}".format(new_lsn))
> 

Re: [Tarantool-patches] [PATCH v1 2/4] test: fix xlog-py/big_lsn.test.py

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

On 14.01.2021 12:02, Leonid Vasiliev wrote:
> Hi! Thank you for the patch.
> LGTM.
>
> On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
>> From: Sergey Bronnikov <estetus@gmail.com>
>>
>> Set correct encoding on opening file.
>>
>> Part of #5538
>
> AFAIU, if the task has been closed, new patches corresponding with the
> task are marked as "Follows up".
>
Fixed!


>> ---
>>   test/xlog-py/big_lsn.test.py | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/test/xlog-py/big_lsn.test.py b/test/xlog-py/big_lsn.test.py
>> index edc1e5620..73dcf7a48 100644
>> --- a/test/xlog-py/big_lsn.test.py
>> +++ b/test/xlog-py/big_lsn.test.py
>> @@ -1,4 +1,5 @@
>>   import os
>> +import codecs
>>     #
>>   # Check that Tarantool handles huge LSNs well (gh-4033).
>> @@ -17,7 +18,7 @@ new_lsn = 123456789123
>>   wal_dir = os.path.join(server.vardir, server.name)
>>   old_wal = os.path.join(wal_dir, "%020d.xlog" % old_lsn)
>>   new_wal = os.path.join(wal_dir, "%020d.xlog" % new_lsn)
>> -with open(old_wal, "r+") as f:
>> +with codecs.open(old_wal, "r+", encoding = "ISO-8859-1") as f:
>>       s = f.read()
>>       s = s.replace("VClock: {{1: {}}}".format(old_lsn),
>>                     "VClock: {{1: {}}}".format(new_lsn))
>>

Re: [Tarantool-patches] [PATCH v1 2/4] test: fix xlog-py/big_lsn.test.py

[Alexander Turenko via Tarantool-patches]

> @@ -17,7 +18,7 @@ new_lsn = 123456789123
>  wal_dir = os.path.join(server.vardir, server.name)
>  old_wal = os.path.join(wal_dir, "%020d.xlog" % old_lsn)
>  new_wal = os.path.join(wal_dir, "%020d.xlog" % new_lsn)
> -with open(old_wal, "r+") as f:
> +with codecs.open(old_wal, "r+", encoding = "ISO-8859-1") as f:

Nit: PEP-8 requires func(foo='bar') -- without surrounding whitespaces.

I would read the binary file using 'rb+' (to bytes) and convert strings,
which are known to be correct utf-8/ascii to Python strings (unicode)
manually, but your solution should work too (and requires less code).

I was afraid that f.read() will see some bytes that're undefined in the
ISO-8859-1 encoding and it'll lead to an exception like it does for
UTF-8. But no, it just returns u'\x00' or kinda.

Okay so.

Re: [Tarantool-patches] [PATCH v1 2/4] test: fix xlog-py/big_lsn.test.py

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

On 14.01.2021 15:38, Alexander Turenko wrote:
>> @@ -17,7 +18,7 @@ new_lsn = 123456789123
>>   wal_dir = os.path.join(server.vardir, server.name)
>>   old_wal = os.path.join(wal_dir, "%020d.xlog" % old_lsn)
>>   new_wal = os.path.join(wal_dir, "%020d.xlog" % new_lsn)
>> -with open(old_wal, "r+") as f:
>> +with codecs.open(old_wal, "r+", encoding = "ISO-8859-1") as f:
> Nit: PEP-8 requires func(foo='bar') -- without surrounding whitespaces.
Agree. Fixed in a branch.
>
> I would read the binary file using 'rb+' (to bytes) and convert strings,
> which are known to be correct utf-8/ascii to Python strings (unicode)
> manually, but your solution should work too (and requires less code).
>
> I was afraid that f.read() will see some bytes that're undefined in the
> ISO-8859-1 encoding and it'll lead to an exception like it does for
> UTF-8. But no, it just returns u'\x00' or kinda.
>
> Okay so.

[Tarantool-patches] [PATCH v1 3/4] test: enable SO_REUSEADDR on socket in httpd.py

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

TL;DR httpd.py warns that port is busy when restart server. With socket
option SO_REUSEADDR it allows to restart httpd.py without problem.

Socket option SO_REUSEADDR tells the kernel that even if this port is
busy, go ahead and reuse it anyway. If it is busy, but with another
state, you will still get an address already in use error. It is useful
if your server has been shut down, and then restarted right away while
sockets are still active on its port.
---
 test/app-tap/httpd.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/app-tap/httpd.py b/test/app-tap/httpd.py
index 62435e91a..60084bdbf 100755
--- a/test/app-tap/httpd.py
+++ b/test/app-tap/httpd.py
@@ -138,6 +138,7 @@ else:
     usage()
 
 sock = socket.socket(sock_family, socket.SOCK_STREAM)
+sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
 sock.bind(sock_addr)
 sock.listen(10)
 
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v1 3/4] test: enable SO_REUSEADDR on socket in httpd.py

[Cyrill Gorcunov via Tarantool-patches]

On Wed, Jan 13, 2021 at 11:48:34AM +0300, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> TL;DR httpd.py warns that port is busy when restart server. With socket
> option SO_REUSEADDR it allows to restart httpd.py without problem.
> 
> Socket option SO_REUSEADDR tells the kernel that even if this port is
> busy, go ahead and reuse it anyway. If it is busy, but with another
> state, you will still get an address already in use error. It is useful
> if your server has been shut down, and then restarted right away while
> sockets are still active on its port.
Ack.

We've been using same techique for criu tests.

Re: [Tarantool-patches] [PATCH v1 3/4] test: enable SO_REUSEADDR on socket in httpd.py

[Leonid Vasiliev via Tarantool-patches]

Hi! Thank you for the patch.
LGTM.

This is a popular technique.
But I don't understand:"Why is it needed when python3 is used, but not 
needed for python2?" If you know, share with me, if not, let it remain a
mystery.

On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> TL;DR httpd.py warns that port is busy when restart server. With socket
> option SO_REUSEADDR it allows to restart httpd.py without problem.
> 
> Socket option SO_REUSEADDR tells the kernel that even if this port is
> busy, go ahead and reuse it anyway. If it is busy, but with another
> state, you will still get an address already in use error. It is useful
> if your server has been shut down, and then restarted right away while
> sockets are still active on its port.
> ---
>   test/app-tap/httpd.py | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/test/app-tap/httpd.py b/test/app-tap/httpd.py
> index 62435e91a..60084bdbf 100755
> --- a/test/app-tap/httpd.py
> +++ b/test/app-tap/httpd.py
> @@ -138,6 +138,7 @@ else:
>       usage()
>   
>   sock = socket.socket(sock_family, socket.SOCK_STREAM)
> +sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>   sock.bind(sock_addr)
>   sock.listen(10)
>   
> 

Re: [Tarantool-patches] [PATCH v1 3/4] test: enable SO_REUSEADDR on socket in httpd.py

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

On 14.01.2021 12:09, Leonid Vasiliev wrote:
> Hi! Thank you for the patch.
> LGTM.
>
> This is a popular technique.
> But I don't understand:"Why is it needed when python3 is used, but not 
> needed for python2?" If you know, share with me, if not, let it remain a
> mystery.

Last two commits are not related to python 3 support and I didn't state it.

These patches are useful changes that I decided to add to the series.

>
> On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> TL;DR httpd.py warns that port is busy when restart server. With socket
>> option SO_REUSEADDR it allows to restart httpd.py without problem.
>>
>> Socket option SO_REUSEADDR tells the kernel that even if this port is
>> busy, go ahead and reuse it anyway. If it is busy, but with another
>> state, you will still get an address already in use error. It is useful
>> if your server has been shut down, and then restarted right away while
>> sockets are still active on its port.
>> ---
>>   test/app-tap/httpd.py | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/test/app-tap/httpd.py b/test/app-tap/httpd.py
>> index 62435e91a..60084bdbf 100755
>> --- a/test/app-tap/httpd.py
>> +++ b/test/app-tap/httpd.py
>> @@ -138,6 +138,7 @@ else:
>>       usage()
>>     sock = socket.socket(sock_family, socket.SOCK_STREAM)
>> +sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
>>   sock.bind(sock_addr)
>>   sock.listen(10)
>>

Re: [Tarantool-patches] [PATCH v1 3/4] test: enable SO_REUSEADDR on socket in httpd.py

[Alexander Turenko via Tarantool-patches]

On Thu, Jan 14, 2021 at 12:57:21PM +0300, Sergey Bronnikov wrote:
> Thanks for review!
> 
> On 14.01.2021 12:09, Leonid Vasiliev wrote:
> > Hi! Thank you for the patch.
> > LGTM.
> > 
> > This is a popular technique.
> > But I don't understand:"Why is it needed when python3 is used, but not
> > needed for python2?" If you know, share with me, if not, let it remain a
> > mystery.
> 
> Last two commits are not related to python 3 support and I didn't state it.
> 
> These patches are useful changes that I decided to add to the series.

I would suggest to send such patches separately in a future to avoid
such confusion.

[Tarantool-patches] [PATCH v1 4/4] test: enable disabled testcases back

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

In a commit "httpc: temporary disable redirecting test case"
(4cc6978ae7be7dc3785a52c064b07850582522de) a set of testcases
has been disabled while the bug #4180 is open.
Right now bug is closed so it is a time to enable testcases back.
---
 test/app-tap/http_client.test.lua | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua
index 093af8fe4..aafad0004 100755
--- a/test/app-tap/http_client.test.lua
+++ b/test/app-tap/http_client.test.lua
@@ -62,7 +62,7 @@ local function stop_server(test, server)
 end
 
 local function test_http_client(test, url, opts)
-    test:plan(11)
+    test:plan(12)
 
     -- gh-4136: confusing httpc usage error message
     local ok, err = pcall(client.request, client)
@@ -85,9 +85,6 @@ local function test_http_client(test, url, opts)
     local r = client.request('GET', url, nil, opts)
     test:is(r.status, 200, 'request')
 
-    -- XXX: enable after resolving of gh-4180: httpc: redirects
-    -- are broken with libcurl-7.30 and older
-    --[[
     -- gh-4119: specify whether to follow 'Location' header
     test:test('gh-4119: follow location', function(test)
         test:plan(7)
@@ -111,7 +108,6 @@ local function test_http_client(test, url, opts)
         test:is(r.body, 'redirecting', 'do not follow location: body')
         test:is(r.headers['location'], '/', 'do not follow location: header')
     end)
-    ]]--
 end
 
 --
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v1 4/4] test: enable disabled testcases back

[Leonid Vasiliev via Tarantool-patches]

Hi! Thank you for the patch.
LGTM.

On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> In a commit "httpc: temporary disable redirecting test case"
> (4cc6978ae7be7dc3785a52c064b07850582522de) a set of testcases
> has been disabled while the bug #4180 is open.
> Right now bug is closed so it is a time to enable testcases back.
> ---
>   test/app-tap/http_client.test.lua | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua
> index 093af8fe4..aafad0004 100755
> --- a/test/app-tap/http_client.test.lua
> +++ b/test/app-tap/http_client.test.lua
> @@ -62,7 +62,7 @@ local function stop_server(test, server)
>   end
>   
>   local function test_http_client(test, url, opts)
> -    test:plan(11)
> +    test:plan(12)
>   
>       -- gh-4136: confusing httpc usage error message
>       local ok, err = pcall(client.request, client)
> @@ -85,9 +85,6 @@ local function test_http_client(test, url, opts)
>       local r = client.request('GET', url, nil, opts)
>       test:is(r.status, 200, 'request')
>   
> -    -- XXX: enable after resolving of gh-4180: httpc: redirects
> -    -- are broken with libcurl-7.30 and older
> -    --[[
>       -- gh-4119: specify whether to follow 'Location' header
>       test:test('gh-4119: follow location', function(test)
>           test:plan(7)
> @@ -111,7 +108,6 @@ local function test_http_client(test, url, opts)
>           test:is(r.body, 'redirecting', 'do not follow location: body')
>           test:is(r.headers['location'], '/', 'do not follow location: header')
>       end)
> -    ]]--
>   end
>   
>   --
> 

Re: [Tarantool-patches] [PATCH v1 0/4] Support Python 3 in Tarantool tests

[Cyrill Gorcunov via Tarantool-patches]

On Wed, Jan 13, 2021 at 11:48:28AM +0300, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <estetus@gmail.com>
> 
> It's another part of fixes related to Python 3 support.  First one was here
> [1]. I haven't create new issue because the task is the same - switch test
> suite to Python 3, so two related commits in a patch series marked with "Part
> of #5538" and no one commit closes any task as it is required by guidelines.
> 
> To test fixes one can use test-run's branch with Python 3 support [2].
> 
> 1. https://lists.tarantool.org/pipermail/tarantool-patches/2020-December/021151.html
> 2. https://github.com/tarantool/test-run/pull/263

I can't comment the code itself since I'm not familiar with test-run ideas but
I use *your* branch to run my tests, and support python3 is critical for me.

Thus fwiw

Tested-by: Cyrill Gorcunov <gorcunov@gmail.com>

Re: [Tarantool-patches] [PATCH v1 0/4] Support Python 3 in Tarantool tests

[Leonid Vasiliev via Tarantool-patches]

Hi! Thank you for the patchset.
LGTM.

On 13.01.2021 11:48, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <estetus@gmail.com>
> 
> It's another part of fixes related to Python 3 support.  First one was here
> [1]. I haven't create new issue because the task is the same - switch test
> suite to Python 3, so two related commits in a patch series marked with "Part
> of #5538" and no one commit closes any task as it is required by guidelines.
> 
> To test fixes one can use test-run's branch with Python 3 support [2].
> 
> 1. https://lists.tarantool.org/pipermail/tarantool-patches/2020-December/021151.html
> 2. https://github.com/tarantool/test-run/pull/263
> 
> Issue: https://github.com/tarantool/tarantool/issues/5538 (closed)
> Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/240927581
> 
> Sergey Bronnikov (4):
>    test: fix app-tap/http_client.test.lua
>    test: fix xlog-py/big_lsn.test.py
>    test: enable SO_REUSEADDR on socket in httpd.py
>    test: enable disabled testcases back
> 
>   test/app-tap/http_client.test.lua |  6 +-----
>   test/app-tap/httpd.py             | 21 +++++++++++----------
>   test/xlog-py/big_lsn.test.py      |  3 ++-
>   3 files changed, 14 insertions(+), 16 deletions(-)
> 

Re: [Tarantool-patches] [PATCH v1 0/4] Support Python 3 in Tarantool tests

[Kirill Yukhin via Tarantool-patches]

Hello,

On 13 янв 11:48, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <estetus@gmail.com>
> 
> It's another part of fixes related to Python 3 support.  First one was here
> [1]. I haven't create new issue because the task is the same - switch test
> suite to Python 3, so two related commits in a patch series marked with "Part
> of #5538" and no one commit closes any task as it is required by guidelines.
> 
> To test fixes one can use test-run's branch with Python 3 support [2].
> 
> 1. https://lists.tarantool.org/pipermail/tarantool-patches/2020-December/021151.html
> 2. https://github.com/tarantool/test-run/pull/263
> 
> Issue: https://github.com/tarantool/tarantool/issues/5538 (closed)
> Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/240927581

I've checked your patchset into 1.10 (3 patches), 2.6, 2.7 and master.

--
Regards, Kirill Yukhin