Re: contrib/spoa/python: A few doc typo and bug fixes

2020-12-08 Thread Gilchrist DADAGLO
No issue with backporting to 2.0. I just mentioned 2.2 as it's the last.

Thanks.

Gilchrist

On Tue, Dec 8, 2020, 16:01 Christopher Faulet  wrote:

> Le 08/12/2020 à 15:37, Gilchrist Dadaglo a écrit :
> > Hi Team,
> >  Please find here-after a few patches for SPOA python module; mainly
> memory related and a
> > couple documentation rewrites. I put them under test for a few months
> now and no additional issue to report so far.
> > Could you please help merge them to master?
> > Any chance they can be backorted to 2.2 (LTS)?
> >
>
> Thanks ! I will handle it. Any reason to not backport these patches as far
> as 2.0 ?
>
> --
> Christopher Faulet
>


[PATCH 8/8] BUG/MEDIUM: spoa/python: Fixing references to None

2020-12-08 Thread Gilchrist Dadaglo
As per https://docs.python.org/3/c-api/none.html, None has to be treated
exactly like other objects for reference counting.
So, when we use it, we need to INCREF and when we are done, DECREF
---
 contrib/spoa_server/ps_python.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index 20861d6..04b21f1 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -634,6 +634,7 @@ static int ps_python_exec_message(struct worker *w, void 
*ref, int nargs, struct
 
switch (args[i].value.type) {
case SPOE_DATA_T_NULL:
+   Py_INCREF(Py_None);
value = Py_None;
break;
case SPOE_DATA_T_BOOL:
@@ -722,6 +723,7 @@ static int ps_python_exec_message(struct worker *w, void 
*ref, int nargs, struct
value = 
PY_BYTES_FROM_STRING_AND_SIZE(args[i].value.u.buffer.str, 
args[i].value.u.buffer.len);
break;
default:
+   Py_INCREF(Py_None);
value = Py_None;
break;
}
@@ -786,9 +788,7 @@ static int ps_python_exec_message(struct worker *w, void 
*ref, int nargs, struct
PyErr_Print();
return 0;
}
-   if (result != Py_None) {
-   Py_DECREF(result);
-   }
+   Py_DECREF(result);
 
return 1;
 }
-- 
2.23.3




[PATCH 4/8] DOC/MINOR: spoa/python: Fixing typos in comments

2020-12-08 Thread Gilchrist Dadaglo
Fixing a missing letter in a comment
---
 contrib/spoa_server/ps_python.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index 380d5b3..fbaa414 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -591,7 +591,7 @@ static int ps_python_exec_message(struct worker *w, void 
*ref, int nargs, struct
return 0;
}
 
-   /* Create th value entry */
+   /* Create the value entry */
 
key = PY_STRING_FROM_STRING("value");
if (key == NULL) {
-- 
2.23.3




[PATCH 2/8] DOC/MINOR: spoa/python: Fixing typo in IP related error messages

2020-12-08 Thread Gilchrist Dadaglo
This commit fixes typos in the ps_python_set_var_ip* byte manipulation error
messages
---
 contrib/spoa_server/ps_python.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index 81bb932..ec97f30 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -236,7 +236,7 @@ static PyObject *ps_python_set_var_ipv4(PyObject *self, 
PyObject *args)
if (value == NULL)
return NULL;
if (PY_STRING_GET_SIZE(value) != sizeof(ip)) {
-   PyErr_Format(spoa_error, "UPv6 manipulation internal error");
+   PyErr_Format(spoa_error, "IPv4 manipulation internal error");
return NULL;
}
memcpy(, PY_STRING_AS_STRING(value), PY_STRING_GET_SIZE(value));
@@ -273,7 +273,7 @@ static PyObject *ps_python_set_var_ipv6(PyObject *self, 
PyObject *args)
if (value == NULL)
return NULL;
if (PY_STRING_GET_SIZE(value) != sizeof(ip)) {
-   PyErr_Format(spoa_error, "UPv6 manipulation internal error");
+   PyErr_Format(spoa_error, "IPv6 manipulation internal error");
return NULL;
}
memcpy(, PY_STRING_AS_STRING(value), PY_STRING_GET_SIZE(value));
-- 
2.23.3




[PATCH 6/8] BUG/MINOR: spoa/python: Cleanup ipaddress objects if initialization fails

2020-12-08 Thread Gilchrist Dadaglo
This change is to ensure objects from the ipaddress module are cleaned
up when spoa module initialization fails.
In general the interpreter would just crash, but in a code where import
is conditional (try/except), then we would keep those objects around
---
 contrib/spoa_server/ps_python.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index 12953f3..f2ddc16 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -410,18 +410,24 @@ static int ps_python_start_worker(struct worker *w)
 
ipv4_address = PyObject_GetAttrString(module_ipaddress, "IPv4Address");
if (ipv4_address == NULL) {
+   Py_DECREF(module_ipaddress);
PyErr_Print();
return 0;
}
 
ipv6_address = PyObject_GetAttrString(module_ipaddress, "IPv6Address");
if (ipv6_address == NULL) {
+   Py_DECREF(ipv4_address);
+   Py_DECREF(module_ipaddress);
PyErr_Print();
return 0;
}
 
PY_INIT_MODULE(m, "spoa", spoa_methods, _module_definition);
if (m == NULL) {
+   Py_DECREF(ipv4_address);
+   Py_DECREF(ipv6_address);
+   Py_DECREF(module_ipaddress);
PyErr_Print();
return 0;
}
-- 
2.23.3




[PATCH 7/8] BUG/MEDIUM: spoa/python: Fixing PyObject_Call positional arguments

2020-12-08 Thread Gilchrist Dadaglo
As per https://docs.python.org/3/c-api/object.html#c.PyObject_Call,
positional arguments should be an empty tuple when not used.
Previously the code had a dictionary instead of tuple. This commit is to
fix it and use tuple to avoid unexpected consequences
---
 contrib/spoa_server/ps_python.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index f2ddc16..20861d6 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -43,7 +43,7 @@ static PyObject *module_ipaddress;
 static PyObject *ipv4_address;
 static PyObject *ipv6_address;
 static PyObject *spoa_error;
-static PyObject *empty_array;
+static PyObject *empty_tuple;
 static struct worker *worker;
 
 static int ps_python_start_worker(struct worker *w);
@@ -522,8 +522,8 @@ static int ps_python_start_worker(struct worker *w)
return 0;
}
 
-   empty_array = PyDict_New();
-   if (empty_array == NULL) {
+   empty_tuple = PyTuple_New(0);
+   if (empty_tuple == NULL) {
PyErr_Print();
return 0;
}
@@ -710,7 +710,7 @@ static int ps_python_exec_message(struct worker *w, void 
*ref, int nargs, struct
PyErr_Print();
return 0;
}
-   value = PyObject_Call(func, empty_array, ip_dict);
+   value = PyObject_Call(func, empty_tuple, ip_dict);
Py_DECREF(func);
Py_DECREF(ip_dict);
break;
@@ -780,7 +780,7 @@ static int ps_python_exec_message(struct worker *w, void 
*ref, int nargs, struct
return 0;
}
 
-   result = PyObject_Call(python_ref, empty_array, fkw);
+   result = PyObject_Call(python_ref, empty_tuple, fkw);
Py_DECREF(fkw);
if (result == NULL) {
PyErr_Print();
-- 
2.23.3




[PATCH 3/8] DOC/MINOR: spoa/python: Rephrasing memory related error messages

2020-12-08 Thread Gilchrist Dadaglo
The old message "No more space left available" was redundant with "left
available". This commit is to rephrase that sentence and make it more
explicit we are talking about memory
---
 contrib/spoa_server/ps_python.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index ec97f30..380d5b3 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -106,7 +106,7 @@ static PyObject *ps_python_set_var_null(PyObject *self, 
PyObject *args)
if (name_len_i == -1)
return NULL;
if (!set_var_null(worker, name, name_len_i, scope)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
Py_RETURN_NONE;
@@ -126,7 +126,7 @@ static PyObject *ps_python_set_var_boolean(PyObject *self, 
PyObject *args)
if (name_len_i == -1)
return NULL;
if (!set_var_bool(worker, name, name_len_i, scope, value)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
Py_RETURN_NONE;
@@ -146,7 +146,7 @@ static PyObject *ps_python_set_var_int32(PyObject *self, 
PyObject *args)
if (name_len_i == -1)
return NULL;
if (!set_var_int32(worker, name, name_len_i, scope, value)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
Py_RETURN_NONE;
@@ -166,7 +166,7 @@ static PyObject *ps_python_set_var_uint32(PyObject *self, 
PyObject *args)
if (name_len_i == -1)
return NULL;
if (!set_var_uint32(worker, name, name_len_i, scope, value)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
Py_RETURN_NONE;
@@ -186,7 +186,7 @@ static PyObject *ps_python_set_var_int64(PyObject *self, 
PyObject *args)
if (name_len_i == -1)
return NULL;
if (!set_var_int64(worker, name, name_len_i, scope, value)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
Py_RETURN_NONE;
@@ -206,7 +206,7 @@ static PyObject *ps_python_set_var_uint64(PyObject *self, 
PyObject *args)
if (name_len_i == -1)
return NULL;
if (!set_var_uint64(worker, name, name_len_i, scope, value)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
Py_RETURN_NONE;
@@ -241,7 +241,7 @@ static PyObject *ps_python_set_var_ipv4(PyObject *self, 
PyObject *args)
}
memcpy(, PY_STRING_AS_STRING(value), PY_STRING_GET_SIZE(value));
if (!set_var_ipv4(worker, name, name_len_i, scope, )) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
/* Once we set the IP value in the worker, we don't need it anymore... 
*/
@@ -278,7 +278,7 @@ static PyObject *ps_python_set_var_ipv6(PyObject *self, 
PyObject *args)
}
memcpy(, PY_STRING_AS_STRING(value), PY_STRING_GET_SIZE(value));
if (!set_var_ipv6(worker, name, name_len_i, scope, )) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
/* Once we set the IP value in the worker, we don't need it anymore... 
*/
@@ -303,7 +303,7 @@ static PyObject *ps_python_set_var_str(PyObject *self, 
PyObject *args)
if (name_len_i == -1 || value_len_i == -1)
return NULL;
if (!set_var_string(worker, name, name_len_i, scope, value, 
value_len_i)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
return NULL;
}
Py_RETURN_NONE;
@@ -326,7 +326,7 @@ static PyObject *ps_python_set_var_bin(PyObject *self, 
PyObject *args)
if (name_len_i == -1 || value_len_i == -1)
return NULL;
if (!set_var_bin(worker, name, name_len_i, scope, value, value_len_i)) {
-   PyErr_SetString(spoa_error, "No space left available");
+   PyErr_SetString(spoa_error, "No more memory space available");
 

[PATCH 5/8] BUG/MINOR: spoa/python: Cleanup references for failed Module Addobject operations

2020-12-08 Thread Gilchrist Dadaglo
As per https://docs.python.org/3/c-api/module.html#c.PyModule_AddObject,
references are stolen by the function only for success. We must do
cleanup manually if there is a failure
---
 contrib/spoa_server/ps_python.c | 27 ++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index fbaa414..12953f3 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -427,8 +427,19 @@ static int ps_python_start_worker(struct worker *w)
}
 
spoa_error = PyErr_NewException("spoa.error", NULL, NULL);
+/* PyModule_AddObject will steal the reference to spoa_error
+* in case of success only
+* We need to increment the counters to continue using it
+* but cleanup in case of failure
+*/
Py_INCREF(spoa_error);
-   PyModule_AddObject(m, "error", spoa_error);
+   ret = PyModule_AddObject(m, "error", spoa_error);
+   if (ret == -1) {
+   Py_DECREF(m);
+   Py_DECREF(spoa_error);
+   PyErr_Print();
+   return 0;
+   }
 
 
value = PyLong_FromLong(SPOE_SCOPE_PROC);
@@ -439,54 +450,68 @@ static int ps_python_start_worker(struct worker *w)
 
ret = PyModule_AddObject(m, "scope_proc", value);
if (ret == -1) {
+   Py_DECREF(m);
+   Py_DECREF(value);
PyErr_Print();
return 0;
}
 
value = PyLong_FromLong(SPOE_SCOPE_SESS);
if (value == NULL) {
+   Py_DECREF(m);
PyErr_Print();
return 0;
}
 
ret = PyModule_AddObject(m, "scope_sess", value);
if (ret == -1) {
+   Py_DECREF(m);
+   Py_DECREF(value);
PyErr_Print();
return 0;
}
 
value = PyLong_FromLong(SPOE_SCOPE_TXN);
if (value == NULL) {
+   Py_DECREF(m);
PyErr_Print();
return 0;
}
 
ret = PyModule_AddObject(m, "scope_txn", value);
if (ret == -1) {
+   Py_DECREF(m);
+   Py_DECREF(value);
PyErr_Print();
return 0;
}
 
value = PyLong_FromLong(SPOE_SCOPE_REQ);
if (value == NULL) {
+   Py_DECREF(m);
PyErr_Print();
return 0;
}
 
ret = PyModule_AddObject(m, "scope_req", value);
if (ret == -1) {
+   Py_DECREF(m);
+   Py_DECREF(value);
PyErr_Print();
return 0;
}
 
value = PyLong_FromLong(SPOE_SCOPE_RES);
if (value == NULL) {
+   Py_DECREF(m);
PyErr_Print();
return 0;
}
 
ret = PyModule_AddObject(m, "scope_res", value);
if (ret == -1) {
+   Py_DECREF(m);
+   Py_DECREF(value);
PyErr_Print();
return 0;
}
-- 
2.23.3




[PATCH 1/8] BUG/MAJOR: spoa/python: Fixing return None

2020-12-08 Thread Gilchrist Dadaglo
As per https://docs.python.org/3/c-api/none.html, None requires to be
incremented before being returned to prevent deallocating none
---
 contrib/spoa_server/ps_python.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/contrib/spoa_server/ps_python.c b/contrib/spoa_server/ps_python.c
index 5cb7ca8..81bb932 100644
--- a/contrib/spoa_server/ps_python.c
+++ b/contrib/spoa_server/ps_python.c
@@ -90,7 +90,7 @@ static PyObject *ps_python_register_message(PyObject *self, 
PyObject *args)
 
ps_register_message(_python_bindings, name, (void *)ref);
 
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_null(PyObject *self, PyObject *args)
@@ -109,7 +109,7 @@ static PyObject *ps_python_set_var_null(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_boolean(PyObject *self, PyObject *args)
@@ -129,7 +129,7 @@ static PyObject *ps_python_set_var_boolean(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_int32(PyObject *self, PyObject *args)
@@ -149,7 +149,7 @@ static PyObject *ps_python_set_var_int32(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_uint32(PyObject *self, PyObject *args)
@@ -169,7 +169,7 @@ static PyObject *ps_python_set_var_uint32(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_int64(PyObject *self, PyObject *args)
@@ -189,7 +189,7 @@ static PyObject *ps_python_set_var_int64(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_uint64(PyObject *self, PyObject *args)
@@ -209,7 +209,7 @@ static PyObject *ps_python_set_var_uint64(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_ipv4(PyObject *self, PyObject *args)
@@ -246,7 +246,7 @@ static PyObject *ps_python_set_var_ipv4(PyObject *self, 
PyObject *args)
}
/* Once we set the IP value in the worker, we don't need it anymore... 
*/
Py_XDECREF(value);
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_ipv6(PyObject *self, PyObject *args)
@@ -283,7 +283,7 @@ static PyObject *ps_python_set_var_ipv6(PyObject *self, 
PyObject *args)
}
/* Once we set the IP value in the worker, we don't need it anymore... 
*/
Py_XDECREF(value);
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_str(PyObject *self, PyObject *args)
@@ -306,7 +306,7 @@ static PyObject *ps_python_set_var_str(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 static PyObject *ps_python_set_var_bin(PyObject *self, PyObject *args)
@@ -329,7 +329,7 @@ static PyObject *ps_python_set_var_bin(PyObject *self, 
PyObject *args)
PyErr_SetString(spoa_error, "No space left available");
return NULL;
}
-   return Py_None;
+   Py_RETURN_NONE;
 }
 
 
-- 
2.23.3




contrib/spoa/python: A few doc typo and bug fixes

2020-12-08 Thread Gilchrist Dadaglo
Hi Team,
Please find here-after a few patches for SPOA python module; mainly memory 
related and a
couple documentation rewrites. I put them under test for a few months now and 
no additional issue to report so far.
Could you please help merge them to master?
Any chance they can be backorted to 2.2 (LTS)?

Thanks
Gilchrist





Re: [PATCH 1/6] MINOR: spoa: allow MAX_FRAME_SIZE override

2020-09-02 Thread Gilchrist DADAGLO
Thanks Christopher,
Appreciate the fast merge.
Gilchrist

On Tue, Sep 1, 2020, 18:36 Christopher Faulet  wrote:

> Le 24/08/2020 à 21:21, gilchr...@dadaglo.com a écrit :
> > From: Bertrand Jacquin 
> >
> > MAX_FRAME_SIZE is forced to the default value of tune.bufsize, however
> > they don't necessarily have to be tight together.
> > ---
> >   contrib/spoa_server/spoa.h | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/contrib/spoa_server/spoa.h b/contrib/spoa_server/spoa.h
> > index 8f912e4..8d6d4be 100644
> > --- a/contrib/spoa_server/spoa.h
> > +++ b/contrib/spoa_server/spoa.h
> > @@ -17,7 +17,10 @@
> >   #include 
> >   #include 
> >
> > +#ifndef MAX_FRAME_SIZE
> >   #define MAX_FRAME_SIZE16384
> > +#endif
> > +
> >   #define SPOP_VERSION  "2.0"
> >   #define SPOA_CAPABILITIES ""
> >
> >
>
> All the series applied ! I've relabeled patches as part of
> "contrib/spoa-server"
> and I've mentioned that all fixes must be backported as far as 2.0.
>
> Thanks,
> --
> Christopher Faulet
>


Re: [PATCH] MAJOR: contrib: porting spoa_server to support python3

2020-05-11 Thread Gilchrist DADAGLO
Hi,
Thank you for the fast merge Willy and Thierry.
I will keep this process in mind for any future contribution.

Cheers,
Gilchrist

On Mon, May 11, 2020, 11:01 Willy Tarreau  wrote:

> On Mon, May 11, 2020 at 10:32:27AM +0200, Thierry Fournier wrote:
> > Hi, thanks Willy,
> >
> > This a good idea to support python3.
> >
> > In facts, python2 is no longer supported, but it is not ready to
> disappear
> > because a lot of scripts in many enterprises are written in python2
> > (exemple: Centos7/RHEL7 the package manager yum is written in python2).
> >
> > In other way the spoe-server is newer and I guess it is not so used, so
> > I should be a good idea to move to python3.
> >
> > So, its ok for me.
>
> Thanks for the quick response. So that's now merged :-)
>
> Gilchrist, thank you for this really clean work and the extensive
> tests you've run on it.
>
> If we notice there's more contribution to this code, maybe we'll have to
> push one step further like we did to support many openssl versions, which
> consists in always sticking to the most recent API and emulate it for the
> old ones. In your case that would for example mean that instead of having
> this :
>
>   -  value = PyString_FromStringAndSize(args[i].name.str,
> args[i].name.len);
>   +  value = PY_STRING_FROM_STRING_AND_SIZE(args[i].name.str,
> args[i].name.len);
>
> You would switch it to "PyUnicode_FromStringAndSize" (which is the new one
> from python3) and add this in your compatibility layer to keep suppor for
> 2.7:
>
>   #define PyUnicode_FromStringAndSize  PyString_FromStringAndSize
>
> That's something that can almost never be done in a single step, as it's
> the best way to mix everything and cause breakage, but with your work
> this seems to become within reach. But given that I don't know if we
> should expect many contributions there, I don't know if it's worth
> investing more time on it!
>
> Cheers,
> Willy
>


Re: [PATCH] MAJOR: contrib: porting spoa_server to support python3

2020-05-06 Thread Gilchrist DADAGLO
Hi,
It was a manual test.
I used the default example and adapted it with the parameters in "Test
settings".
"Test result" is just a copy from the logs to stdio.

I was just to make sure we exchange all data types supported by the
protocol.

This could be automated indeed but I didn't do anything for that purpose.

Gilchrist

On Wed, May 6, 2020, 16:13 Илья Шипицин  wrote:

> How did you get "test result"?
> Should we add automated test for that? For example, once a week
>
> On Wed, May 6, 2020, 5:28 PM Gilchrist Dadaglo  wrote:
>
>>
>> Background:
>> Python 2 is no longer supported since January, 1st 2020 as per
>> https://www.python.org/doc/sunset-python-2/
>> The purpose of this change is to make the spoa_server contrib
>> library
>> compatible with Python 3 to allow transition to Python 3.
>>
>> Test Settings:
>> ps_python.py:
>> ...
>> spoa.set_var_null("null", spoa.scope_txn)
>> spoa.set_var_boolean("boolean", spoa.scope_txn, True)
>> spoa.set_var_int32("int32", spoa.scope_txn, 1234)
>> spoa.set_var_uint32("uint32", spoa.scope_txn, 1234)
>> spoa.set_var_int64("int64", spoa.scope_txn, 1234)
>> spoa.set_var_uint64("uint64", spoa.scope_txn, 1234)
>> spoa.set_var_ipv4("ipv4", spoa.scope_txn,
>> ipaddress.IPv4Address(u"127.0.0.1"))
>> spoa.set_var_ipv6("ipv6", spoa.scope_txn,
>> ipaddress.IPv6Address(u"1::f"))
>> spoa.set_var_str("str", spoa.scope_txn, "1::f")
>> spoa.set_var_bin("bin", spoa.scope_txn, "1:\x01:\x42f\x63\x63")
>> spoa.set_var_str("python_version", spoa.scope_sess,
>> str(sys.version_info))
>> ...
>> haproxy.cfg:
>> ...
>> http-request capture var(txn.verb.null),debug len 1
>> http-request capture var(txn.verb.boolean),debug len 1
>> http-request capture var(txn.verb.int32),debug len 4
>> http-request capture var(txn.verb.uint32),debug len 4
>> http-request capture var(txn.verb.int64),debug len 4
>> http-request capture var(txn.verb.uint64),debug len 4
>> http-request capture var(txn.verb.ipv4),debug len 16
>> http-request capture var(txn.verb.ipv6),debug len 45
>> http-request capture var(txn.verb.str),debug len 32
>> http-request capture var(txn.verb.bin),debug len 32
>> http-request capture var(sess.verb.python_version),debug len 100
>> ...
>>
>> Test result:
>> Python 3.8:
>> ft_public ft_public/ 0/-1/-1/-1/0 403 212 - - PR--
>> 1/1/0/0/0 0/0
>> {|1|1234|1234|1234|1234|127.0.0.1|1::f|1::f|1:#01:Bfcc|sys.version_info(major=3,
>> minor=8, micro=1, releaselevel='final', serial=0)} "POST / HTTP/1.1"
>> Python 3.7:
>> ft_public ft_public/ 0/-1/-1/-1/0 403 212 - - PR--
>> 1/1/0/0/0 0/0
>> {|1|1234|1234|1234|1234|127.0.0.1|1::f|1::f|1:#01:Bfcc|sys.version_info(major=3,
>> minor=7, micro=6, releaselevel='final', serial=0)} "POST / HTTP/1.1"
>> Python 3.6:
>> ft_public ft_public/ 0/-1/-1/-1/0 403 212 - - PR--
>> 1/1/0/0/0 0/0
>> {|1|1234|1234|1234|1234|127.0.0.1|1::f|1::f|1:#01:Bfcc|sys.version_info(major=3,
>> minor=6, micro=10, releaselevel='final', serial=0)} "POST / HTTP/1.1"
>> Python 2.7:
>> ft_public ft_public/ 0/-1/-1/-1/0 403 212 - - PR--
>> 1/1/0/0/0 0/0
>> {|1|1234|1234|1234|1234|127.0.0.1|1::f|1::f|1:#01:Bfcc|sys.version_info(major=2,
>> minor=7, micro=17, releaselevel='final', serial=0)} "POST / HTTP/1.1"
>>
>> Not tested:
>> Python <2.7
>> ---
>>  haproxy/contrib/spoa_server/Makefile|  37 +
>>  haproxy/contrib/spoa_server/README  |  10 +-
>>  haproxy/contrib/spoa_server/ps_python.c | 179 +++-
>>  haproxy/contrib/spoa_server/ps_python.h |  52 +++
>>  4 files changed, 241 insertions(+), 37 deletions(-)
>>  create mode 100644 haproxy/contrib/spoa_server/ps_python.h
>>
>>


[PATCH] MAJOR: contrib: porting spoa_server to support python3

2020-05-06 Thread Gilchrist Dadaglo
haproxy/contrib/spoa_server/README
index 341f5f9..f654a23 100644
--- a/haproxy/contrib/spoa_server/README
+++ b/haproxy/contrib/spoa_server/README
@@ -14,9 +14,10 @@ is done.
 You have to install the development packages, either from the
 distribution repositories or from the source.
 
-CentOS/RHEL: yum install python-devel
+CentOS/RHEL: sudo yum install python3-devel
 
-The current python version in use is 2.7.
+The current minimal python version compatible with this library is 2.7.
+It's recommended to use python version 3 where possible due to python 2 deprecation.
 
 
   Compilation
@@ -28,6 +29,11 @@ USE_LUA=1 and/or USE_PYTHON=1.
 You can add LUA_INC=.. LUA_LIB=.. to the make command to set the paths to
 the lua header files and lua libraries.
 
+Similarly, you can add PYTHON_INC=.. PYTHON_LIB=.. to the make command to set the paths to
+the python header files and python libraries.
+By default, it will try to compile by detecting the default python 3 parameters.
+It will fall back to python 2 if python 3 is not available.
+
   Start the service
 -
 
diff --git a/haproxy/contrib/spoa_server/ps_python.c b/haproxy/contrib/spoa_server/ps_python.c
index 0a9fbff..019d125 100644
--- a/haproxy/contrib/spoa_server/ps_python.c
+++ b/haproxy/contrib/spoa_server/ps_python.c
@@ -1,13 +1,25 @@
 /* spoa-server: processing Python
  *
  * Copyright 2018 OZON / Thierry Fournier 
+ * Copyright (C) 2020  Gilchrist Dadaglo 
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version
  * 2 of the License, or (at your option) any later version.
  *
+ * This program is provided in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+/*
+ *	Define PY_SSIZE_T_CLEAN before including Python.h
+ *	as per https://docs.python.org/3/c-api/arg.html and https://docs.python.org/2/c-api/arg.html
  */
+#define PY_SSIZE_T_CLEAN
 
 #include 
 
@@ -15,8 +27,10 @@
 
 #include 
 #include 
+#include 
 
 #include "spoa.h"
+#include "ps_python.h"
 
 /* Embedding python documentation:
  *
@@ -43,6 +57,28 @@ static struct ps ps_python_bindings = {
 	.ext = ".py",
 };
 
+static int ps_python_check_overflow(Py_ssize_t len)
+{
+	/* There might be an overflow when converting from Py_ssize_t to int.
+	 * This function will catch those cases.
+	 * Also, spoa "struct chunk" is limited to int size.
+	 * We should not send data bigger than it can handle.
+	 */
+	if (len >= (Py_ssize_t)INT_MAX) {
+		PyErr_Format(spoa_error,
+"%d is over 2GB. Please split in smaller pieces.", \
+len);
+		return -1;
+	} else {
+		return Py_SAFE_DOWNCAST(len, Py_ssize_t, int);
+	}
+}
+
+#if IS_PYTHON_3K
+static PyObject *module_spoa;
+static PyObject *PyInit_spoa_module(void);
+#endif /* IS_PYTHON_3K */
+
 static PyObject *ps_python_register_message(PyObject *self, PyObject *args)
 {
 	const char *name;
@@ -60,12 +96,16 @@ static PyObject *ps_python_register_message(PyObject *self, PyObject *args)
 static PyObject *ps_python_set_var_null(PyObject *self, PyObject *args)
 {
 	const char *name;
-	int name_len;
+	Py_ssize_t name_len;
+	int name_len_i;
 	int scope;
 
 	if (!PyArg_ParseTuple(args, "s#i", , _len, ))
 		return NULL;
-	if (!set_var_null(worker, name, name_len, scope)) {
+	name_len_i = ps_python_check_overflow(name_len);
+	if (name_len_i == -1)
+		return NULL;
+	if (!set_var_null(worker, name, name_len_i, scope)) {
 		PyErr_SetString(spoa_error, "No space left available");
 		return NULL;
 	}
@@ -75,13 +115,17 @@ static PyObject *ps_python_set_var_null(PyObject *self, PyObject *args)
 static PyObject *ps_python_set_var_boolean(PyObject *self, PyObject *args)
 {
 	const char *name;
-	int name_len;
+	Py_ssize_t name_len;
 	int scope;
 	int value;
+	int name_len_i;
 
 	if (!PyArg_ParseTuple(args, "s#ii", , _len, , ))
 		return NULL;
-	if (!set_var_bool(worker, name, name_len, scope, value)) {
+	name_len_i = ps_python_check_overflow(name_len);
+	if (name_len_i == -1)
+		return NULL;
+	if (!set_var_bool(worker, name, name_len_i, scope, value)) {
 		PyErr_SetString(spoa_error, "No space left available");
 		return NULL;
 	}
@@ -91,13 +135,17 @@ static PyObject *ps_python_set_var_boolean(PyObject *self, PyObject *args)
 static PyObject *ps_python_set_var_int32(PyObject *self, PyObject *args)
 {
 	const char *name;
-	int name_len;
+	Py_ssize_t name_len;
 	int scope;
 	int32_t value;
+	int name_len_i;
 
 	if (!PyArg_ParseTuple(args, "s#ii", , _len, , ))
 		return NULL;
-	if (!set_var_int32(worker, name, name_len, scope, value)) {
+	name_len_i = ps_python_check_overflow(name_len);
+	if (name_len_i == -1)
+		return NULL;
+	if (!set_var_int32(worker,