[GitHub] [arrow-datafusion] jorgecarleitao commented on a change in pull request #275: add postgres comparison to python test

[GitBox <gi...@apache.org>]

jorgecarleitao commented on a change in pull request #275:
URL: https://github.com/apache/arrow-datafusion/pull/275#discussion_r627681688



##########
File path: .github/workflows/python_test.yaml
##########
@@ -21,38 +21,57 @@ on: [push, pull_request]
 jobs:
   test:
     runs-on: ubuntu-latest
+    services:
+      postgres:
+        image: postgres:13
+        env:
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: db_test
+        ports:
+          - 5432/tcp
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
     steps:
-    - uses: actions/checkout@v2
-    - name: Setup Rust toolchain
-      run: |
-        rustup toolchain install nightly-2021-01-06
-        rustup default nightly-2021-01-06
-        rustup component add rustfmt
-    - name: Cache Cargo
-      uses: actions/cache@v2
-      with:
-        path: /home/runner/.cargo
-        key: cargo-maturin-cache-
-    - name: Cache Rust dependencies
-      uses: actions/cache@v2
-      with:
-        path: /home/runner/target
-        key: target-maturin-cache-
-    - uses: actions/setup-python@v2
-      with:
-        python-version: '3.7'
-    - name: Install Python dependencies
-      run: python -m pip install --upgrade pip setuptools wheel
-    - name: Run tests
-      run: |
-        cd python/
-        export CARGO_HOME="/home/runner/.cargo"
-        export CARGO_TARGET_DIR="/home/runner/target"
+      - uses: actions/checkout@v2
+      - name: Setup Rust toolchain
+        run: |
+          rustup toolchain install nightly-2021-01-06
+          rustup default nightly-2021-01-06
+          rustup component add rustfmt
+      - name: Cache Cargo
+        uses: actions/cache@v2
+        with:
+          path: /home/runner/.cargo
+          key: cargo-maturin-cache-
+      - name: Cache Rust dependencies
+        uses: actions/cache@v2
+        with:
+          path: /home/runner/target
+          key: target-maturin-cache-
+      - uses: actions/setup-python@v2
+        with:
+          python-version: "3.8"
+      - name: Install Python dependencies
+        run: python -m pip install --upgrade pip setuptools wheel
+      - name: Run tests
+        run: |
+          cd python/
+          export CARGO_HOME="/home/runner/.cargo"
+          export CARGO_TARGET_DIR="/home/runner/target"
 
-        python -m venv venv
-        source venv/bin/activate
+          python -m venv venv
+          source venv/bin/activate
 
-        pip install maturin==0.10.4 toml==0.10.1 pyarrow==4.0.0
-        maturin develop
+          # TODO move to use poetry

Review comment:
       poetry is a python package to resolve dependency trees. Basically `pip` is kind of "broken" in some aspects, and [poetry](https://python-poetry.org/) (together with conda, [pip-env](https://pypi.org/project/pipenv/) and [pip-tools](https://github.com/jazzband/pip-tools)) are addressing some of problems with `pip`.
   
   In this context, it addresses the line below, `pip install ...`.
   
   My understanding is that poetry is used in the context of Python applications, and this is a Python library. I can understand its use for resolving dependencies in testing the library, but something like [`tox`](https://tox.readthedocs.io/en/latest/) would probably make more sense, even though it is not so easy because this is a compiled Python library.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org