THRIFT-5923: UUID python

.github/workflows/build.yml

@@ -577,12 +577,6 @@ jobs:
       - name: Run make install for python
         run: sudo make -C lib/py install
 
-      # - name: Run make install-exec-hook for python
-      #   run: sudo make -C lib/py install-exec-hook
-
-      - name: Run make for python libs
-        run: make -C lib/py
-
       - name: Run make check for python libs
         run: make -C lib/py check
 
@@ -842,7 +836,7 @@ jobs:
         # kotlin cross test are failing -> see THRIFT-5879
         server_lang: ['java', 'go', 'cpp', 'py', 'rb']
         # we always use comma join as many client langs as possible, to reduce the number of jobs
-        client_lang: ['java,kotlin', 'go,cpp', 'py', 'rb']
+        client_lang: ['java,kotlin', 'go,cpp,py', 'rb']
       fail-fast: false
     steps:
       - uses: actions/checkout@v6

LANGUAGES.md

@@ -301,7 +301,7 @@ Thrift's core protocol is TBinary, supported by all languages except for JavaScr
 <!-- Since -----------------><td>0.2.0</td>
 <!-- Build Systems ---------><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Language Levels -------><td>2.7.12, 3.5.2</td><td>2.7.15, 3.6.8</td>
-<!-- Field types -----------><td><img src="/doc/images/cred.png" alt=""/></td>
+<!-- Field types -----------><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Low-Level Transports --><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cred.png" alt=""/></td><td><img src="/doc/images/cred.png" alt=""/></td><td><img src="/doc/images/cred.png" alt=""/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Transport Wrappers ----><td><img src="/doc/images/cred.png" alt=""/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>
 <!-- Protocols -------------><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td><td><img src="/doc/images/cgrn.png" alt="Yes"/></td>

lib/cpp/src/thrift/protocol/TProtocol.h

@@ -784,6 +784,10 @@ uint32_t skip(Protocol_& prot, TType type) {
     result += prot.readListEnd();
     return result;
   }
+  case T_UUID: {
+    TUuid uuid;
+    return prot.readUUID(uuid);
+  }
   default:
     break;
   }

lib/py/setup.py

@@ -85,6 +85,14 @@ def run_setup(with_binary):
                               'src/ext/binary.cpp',
                               'src/ext/compact.cpp',
                           ],
+                          depends=[
+                              'src/ext/binary.h',
+                              'src/ext/compact.h',
+                              'src/ext/endian.h',
+                              'src/ext/protocol.h',
+                              'src/ext/protocol.tcc',
+                              'src/ext/types.h',
+                          ],
                           include_dirs=include_dirs,
                           )
             ],
@@ -138,6 +146,8 @@ def run_setup(with_binary):
 try:
     with_binary = True
     run_setup(with_binary)
+    sys.exit(0)
+
 except BuildFailed:
     print()
     print('*' * 80)
@@ -146,4 +156,16 @@ def run_setup(with_binary):
     print('*' * 80)
     print()
 
+# Retry but without the binary
+try:
     run_setup(False)
+    sys.exit(0)
+
+except BuildFailed:
+    print()
+    print('*' * 80)
+    print("An error occurred while trying to compile without the C extension enabled")
+    print("Build failed")
+    print('*' * 80)
+    print()
+    sys.exit(1)

lib/py/src/Thrift.py

@@ -34,8 +34,7 @@ class TType(object):
     MAP = 13
     SET = 14
     LIST = 15
-    UTF8 = 16
-    UTF16 = 17
+    UUID = 16
 
     _VALUES_TO_NAMES = (
         'STOP',
@@ -54,8 +53,7 @@ class TType(object):
         'MAP',
         'SET',
         'LIST',
-        'UTF8',
-        'UTF16',
+        'UUID',
     )
 
 

lib/py/src/ext/binary.h

@@ -88,6 +88,10 @@ class BinaryProtocol : public ProtocolBase<BinaryProtocol> {
     return encodeValue(value, parsedspec.type, parsedspec.typeargs);
   }
 
+  void writeUuid(char* value) {
+    writeBuffer(value, 16);
+  }
+
   void writeFieldStop() { writeByte(static_cast<uint8_t>(T_STOP)); }
 
   bool readBool(bool& val) {
@@ -159,6 +163,13 @@ class BinaryProtocol : public ProtocolBase<BinaryProtocol> {
     return len;
   }
 
+  int32_t readUuid(char** buf) {
+    if (!readBytes(buf, 16)) {
+      return -1;
+    }
+    return 16;
+  }
+
   int32_t readListBegin(TType& etype) {
     int32_t len;
     uint8_t b = 0;
@@ -206,6 +217,7 @@ class BinaryProtocol : public ProtocolBase<BinaryProtocol> {
     }
     SKIPBYTES(len);
   }
+  bool skipUuid() { SKIPBYTES(16); }
 #undef SKIPBYTES
 
 private:

lib/py/src/ext/compact.cpp

@@ -24,23 +24,26 @@ namespace apache {
 namespace thrift {
 namespace py {
 
+/** Mapping of Compact type to Thrift Type according.
+ * This list must match the TType enum in TEnum.h */
 const uint8_t CompactProtocol::TTypeToCType[] = {
-    CT_STOP,         // T_STOP
-    0,               // unused
-    CT_BOOLEAN_TRUE, // T_BOOL
-    CT_BYTE,         // T_BYTE
-    CT_DOUBLE,       // T_DOUBLE
-    0,               // unused
-    CT_I16,          // T_I16
-    0,               // unused
-    CT_I32,          // T_I32
-    0,               // unused
-    CT_I64,          // T_I64
-    CT_BINARY,       // T_STRING
-    CT_STRUCT,       // T_STRUCT
-    CT_MAP,          // T_MAP
-    CT_SET,          // T_SET
-    CT_LIST,         // T_LIST
+/*  0 */    CT_STOP,         // T_STOP
+/*  1 */    0,               // unused
+/*  2 */    CT_BOOLEAN_TRUE, // T_BOOL
+/*  3 */    CT_BYTE,         // T_BYTE
+/*  4 */    CT_DOUBLE,       // T_DOUBLE
+/*  5 */    0,               // unused
+/*  6 */    CT_I16,          // T_I16
+/*  7 */    0,               // unused
+/*  8 */    CT_I32,          // T_I32
+/*  9 */    0,               // unused
+/* 10 */    CT_I64,          // T_I64
+/* 11 */    CT_BINARY,       // T_STRING
+/* 12 */    CT_STRUCT,       // T_STRUCT
+/* 13 */    CT_MAP,          // T_MAP
+/* 14 */    CT_SET,          // T_SET
+/* 15 */    CT_LIST,         // T_LIST
+/* 16 */    CT_UUID,         // T_UUID
 };
 
 bool CompactProtocol::readFieldBegin(TType& type, int16_t& tag) {
@@ -98,6 +101,8 @@ TType CompactProtocol::getTType(uint8_t type) {
     return T_MAP;
   case CT_STRUCT:
     return T_STRUCT;
+  case CT_UUID:
+    return T_UUID;
   default:
     PyErr_Format(PyExc_TypeError, "don't know what type: %d", type);
     return static_cast<TType>(-1);

lib/py/src/ext/compact.h

@@ -104,6 +104,10 @@ class CompactProtocol : public ProtocolBase<CompactProtocol> {
 
   void writeFieldStop() { writeByte(0); }
 
+  void writeUuid(char* value) {
+    writeBuffer(value, 16);
+  }
+
   bool readBool(bool& val) {
     if (readBool_.exists) {
       readBool_.exists = false;
@@ -231,6 +235,13 @@ class CompactProtocol : public ProtocolBase<CompactProtocol> {
   }
   bool readFieldBegin(TType& type, int16_t& tag);
 
+  bool readUuid(char** buf) {
+    if (!readBytes(buf, 16)) {
+      return false;
+    }
+    return true;
+  }
+
   bool skipBool() {
     bool val;
     return readBool(val);
@@ -263,6 +274,9 @@ class CompactProtocol : public ProtocolBase<CompactProtocol> {
     }
     SKIPBYTES(len);
   }
+  bool skipUuid() {
+    SKIPBYTES(16);
+  }
 #undef SKIPBYTES
 
 private:
@@ -279,7 +293,8 @@ class CompactProtocol : public ProtocolBase<CompactProtocol> {
     CT_LIST = 0x09,
     CT_SET = 0x0A,
     CT_MAP = 0x0B,
-    CT_STRUCT = 0x0C
+    CT_STRUCT = 0x0C,
+    CT_UUID = 0x0D,
   };
 
   static const uint8_t TTypeToCType[];
@@ -288,7 +303,7 @@ class CompactProtocol : public ProtocolBase<CompactProtocol> {
 
   int toCompactType(TType type) {
     int i = static_cast<int>(type);
-    return i < 16 ? TTypeToCType[i] : -1;
+    return i <= 16 ? TTypeToCType[i] : -1;
   }
 
   uint32_t toZigZag(int32_t val) { return (val >> 31) ^ (val << 1); }

lib/py/src/ext/module.cpp

@@ -38,6 +38,8 @@
 PyObject* INTERN_STRING(TFrozenDict);
 PyObject* INTERN_STRING(cstringio_buf);
 PyObject* INTERN_STRING(cstringio_refill);
+PyObject* INTERN_STRING(UUID);
+PyObject* INTERN_STRING(bytes);
 static PyObject* INTERN_STRING(string_length_limit);
 static PyObject* INTERN_STRING(container_length_limit);
 static PyObject* INTERN_STRING(trans);
@@ -186,6 +188,8 @@ void initfastbinary() {
   INIT_INTERN_STRING(string_length_limit);
   INIT_INTERN_STRING(container_length_limit);
   INIT_INTERN_STRING(trans);
+  INIT_INTERN_STRING(UUID);
+  INIT_INTERN_STRING(bytes);
 #undef INIT_INTERN_STRING
 
   PyObject* module =

lib/py/src/ext/protocol.tcc

@@ -542,10 +542,27 @@ bool ProtocolBase<Impl>::encodeValue(PyObject* value, TType type, PyObject* type
     return true;
   }
 
+  case T_UUID: {
+    ScopedPyObject instval(PyObject_GetAttr(value, INTERN_STRING(bytes)));
+    if (!instval) {
+        return false;
+    }
+
+    Py_ssize_t size;
+    char* buffer;
+    if (PyBytes_AsStringAndSize(instval.get(), &buffer, &size) < 0) {
+        return false;
+    }
+    if (size != 16) {
+        PyErr_SetString(PyExc_TypeError, "uuid.bytes must be exactly 16 bytes long");
+        return false;
+    }
+    impl()->writeUuid(buffer);
+    return true;
+  }
+
   case T_STOP:
   case T_VOID:
-  case T_UTF16:
-  case T_UTF8:
   case T_U64:
   default:
     PyErr_Format(PyExc_TypeError, "Unexpected TType for encodeValue: %d", type);
@@ -625,11 +642,12 @@ bool ProtocolBase<Impl>::skip(TType type) {
     }
     return true;
   }
+  case T_UUID: {
+    return impl()->skipUuid();
+  }
 
   case T_STOP:
   case T_VOID:
-  case T_UTF16:
-  case T_UTF8:
   case T_U64:
   default:
     PyErr_Format(PyExc_TypeError, "Unexpected TType for skip: %d", type);
@@ -816,10 +834,36 @@ PyObject* ProtocolBase<Impl>::decodeValue(TType type, PyObject* typeargs) {
     return readStruct(Py_None, parsedargs.klass, parsedargs.spec);
   }
 
+  case T_UUID: {
+    char* buf = nullptr;
+    if(!impl()->readUuid(&buf)) {
+      return nullptr;
+    }
+
+    if(!UuidModule) {
+      UuidModule = PyImport_ImportModule("uuid");
+      if (!UuidModule)
+        return nullptr;
+    }
+
+    ScopedPyObject cls(PyObject_GetAttr(UuidModule, INTERN_STRING(UUID)));
+    if (!cls) {
+        return nullptr;
+    }
+
+    ScopedPyObject pyBytes(PyBytes_FromStringAndSize(buf, 16));
+    if (!pyBytes) {
+        return nullptr;
+    }
+
+    ScopedPyObject args(PyTuple_New(0));
+    ScopedPyObject kwargs(Py_BuildValue("{O:O}", INTERN_STRING(bytes), pyBytes.get()));
+    ScopedPyObject ret(PyObject_Call(cls.get(), args.get(), kwargs.get()));
+    return ret.release();
+  }
+
   case T_STOP:
   case T_VOID:
-  case T_UTF16:
-  case T_UTF8:
   case T_U64:
   default:
     PyErr_Format(PyExc_TypeError, "Unexpected TType for decodeValue: %d", type);

lib/py/src/ext/types.cpp

@@ -26,6 +26,7 @@ namespace thrift {
 namespace py {
 
 PyObject* ThriftModule = nullptr;
+PyObject* UuidModule = nullptr;
 
 #if PY_MAJOR_VERSION < 3
 char refill_signature[] = {'s', '#', 'i'};

lib/py/src/ext/types.h

@@ -48,13 +48,16 @@ extern "C" {
 extern PyObject* INTERN_STRING(TFrozenDict);
 extern PyObject* INTERN_STRING(cstringio_buf);
 extern PyObject* INTERN_STRING(cstringio_refill);
+extern PyObject* INTERN_STRING(UUID);
+extern PyObject* INTERN_STRING(bytes);
 }
 
 namespace apache {
 namespace thrift {
 namespace py {
 
 extern PyObject* ThriftModule;
+extern PyObject* UuidModule;
 
 // Stolen out of TProtocol.h.
 // It would be a huge pain to have both get this from one place.
@@ -76,8 +79,7 @@ enum TType {
   T_MAP = 13,
   T_SET = 14,
   T_LIST = 15,
-  T_UTF8 = 16,
-  T_UTF16 = 17
+  T_UUID = 16,
 };
 
 // replace with unique_ptr when we're OK with C++11