fishy commented on code in PR #2787:
URL: https://github.com/apache/thrift/pull/2787#discussion_r1171613064
##########
.github/workflows/build.yml:
##########
@@ -344,12 +344,79 @@ jobs:
- name: Run make test_recursive for rust
run: make -C lib/rs/test_recursive check
+ lib-python:
+ needs: compiler
+ runs-on: ubuntu-20.04
+ strategy:
+ matrix:
+ python-version: ["2.x", "3.x"]
+ steps:
+ - uses: actions/checkout@v3
+
+ - name: Install dependencies
+ run: |
+ sudo apt-get update -yq
+ sudo apt-get install -y --no-install-recommends $BUILD_DEPS
+ sudo apt-get install -y --no-install-recommends curl openssl
ca-certificates
+
+ - name: Set up Python
+ uses: actions/setup-python@v3
+ with:
+ python-version: ${{ matrix.python-version }}
+
+ - name: Python setup
+ run: |
+ python -m pip install --upgrade pip setuptools wheel flake8 tornado
twisted zope.interface
+ python --version
+ pip --version
+
+ - name: Python 2.x backport setup
+ if: matrix.python-version == '2.x'
+ run: |
+ python -m pip install --upgrade ipaddress
backports.ssl_match_hostname
+
+ - name: Run bootstrap
+ run: ./bootstrap.sh
+
+ - name: Run configure 2.x
+ if: matrix.python-version == '2.x'
+ run: |
+ ./configure $(echo $CONFIG_ARGS_FOR_LIBS | sed
's/without-python/with-python/')
+
+ - name: Run configure 3.x
+ if: matrix.python-version != '2.x'
+ run: |
+ ./configure $(echo $CONFIG_ARGS_FOR_LIBS | sed
's/without-py3/with-py3/')
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: thrift-compiler
+ path: compiler/cpp
+
+ - name: Run thrift-compiler
+ run: |
+ chmod a+x compiler/cpp/thrift
+ compiler/cpp/thrift -version
+
+ - name: Run make for python
+ run: make -C lib/py
+
+ - 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 check for python
+ run: make -C lib/py check
Review Comment:
we actually have two sets of tests, one under `lib/py` (which is the one
here) and another under `test/py`. can you also add a step to run `make -C
test/py check`?
##########
lib/py/src/server/TNonblockingServer.py:
##########
@@ -268,7 +268,7 @@ def prepare(self):
self.socket.listen()
for _ in range(self.threads):
thread = Worker(self.tasks)
- thread.setDaemon(True)
+ thread.daemon = True
Review Comment:
🔕 oh I see `setDaemon` was deprecated since python 3.10.
##########
.github/workflows/build.yml:
##########
@@ -344,12 +344,79 @@ jobs:
- name: Run make test_recursive for rust
run: make -C lib/rs/test_recursive check
+ lib-python:
+ needs: compiler
+ runs-on: ubuntu-20.04
+ strategy:
+ matrix:
+ python-version: ["2.x", "3.x"]
+ steps:
+ - uses: actions/checkout@v3
+
+ - name: Install dependencies
+ run: |
+ sudo apt-get update -yq
+ sudo apt-get install -y --no-install-recommends $BUILD_DEPS
+ sudo apt-get install -y --no-install-recommends curl openssl
ca-certificates
+
+ - name: Set up Python
+ uses: actions/setup-python@v3
+ with:
+ python-version: ${{ matrix.python-version }}
+
+ - name: Python setup
+ run: |
+ python -m pip install --upgrade pip setuptools wheel flake8 tornado
twisted zope.interface
+ python --version
+ pip --version
+
+ - name: Python 2.x backport setup
+ if: matrix.python-version == '2.x'
+ run: |
+ python -m pip install --upgrade ipaddress
backports.ssl_match_hostname
+
+ - name: Run bootstrap
+ run: ./bootstrap.sh
+
+ - name: Run configure 2.x
+ if: matrix.python-version == '2.x'
+ run: |
+ ./configure $(echo $CONFIG_ARGS_FOR_LIBS | sed
's/without-python/with-python/')
+
+ - name: Run configure 3.x
+ if: matrix.python-version != '2.x'
+ run: |
+ ./configure $(echo $CONFIG_ARGS_FOR_LIBS | sed
's/without-py3/with-py3/')
+
+ - uses: actions/download-artifact@v3
+ with:
+ name: thrift-compiler
+ path: compiler/cpp
+
+ - name: Run thrift-compiler
+ run: |
+ chmod a+x compiler/cpp/thrift
+ compiler/cpp/thrift -version
+
+ - name: Run make for python
+ run: make -C lib/py
+
+ - 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 check for python
+ run: make -C lib/py check
+
cross-test:
needs:
- lib-java-kotlin
- lib-swift
- lib-rust
- lib-go
+ - lib-python
Review Comment:
we actually also need to add python (I'm not sure if it's gonna be `py3` or
`python`) to line 483 (`PRECROSS_LANGS`) to actually run the cross tests with
python.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
