Skip to content

Set up github actions + remove databricks-specific test infra #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 24, 2022
Merged

Set up github actions + remove databricks-specific test infra #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b83a2cb
Remove databricks-specific test fixtures
susodapop Jun 7, 2022
14e8229
Introduce pytest, black, and mypy workflows
susodapop Jun 7, 2022
0226b61
Skip test_fetches_bench case
susodapop Jun 7, 2022
0bd92fa
Remove references to databricks-specific testing infra
susodapop Jun 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 148 additions & 0 deletions .github/workflows/code-quality-checks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
name: Code Quality Checks
on: [push]
jobs:
run-tests:
runs-on: ubuntu-latest
steps:
#----------------------------------------------
# check-out repo and set-up python
#----------------------------------------------
- name: Check out repository
uses: actions/checkout@v2
- name: Set up python
id: setup-python
uses: actions/setup-python@v2
with:
python-version: 3.7
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to extend the test matrix to cover other supported versions?
3.8 or 3.9?

which versions do we officially support?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's easy enough to duplicate this workflow for Python 3.8 through 3.10.

I think to minimise duplication we can have one workflow that uses the latest version of python and another that uses the oldest version we support.

#----------------------------------------------
# ----- install & configure poetry -----
#----------------------------------------------
- name: Install Poetry
uses: snok/install-poetry@v1
with:
virtualenvs-create: true
virtualenvs-in-project: true
installer-parallel: true

#----------------------------------------------
# load cached venv if cache exists
#----------------------------------------------
- name: Load cached venv
id: cached-poetry-dependencies
uses: actions/cache@v2
with:
path: .venv
key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ github.event.repository.name }}-${{ hashFiles('**/poetry.lock') }}
#----------------------------------------------
# install dependencies if cache does not exist
#----------------------------------------------
- name: Install dependencies
if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
run: poetry install --no-interaction --no-root
#----------------------------------------------
# install your root project, if required
#----------------------------------------------
- name: Install library
run: poetry install --no-interaction
#----------------------------------------------
# run test suite
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@susodapop
which of the following is true:

  1. does this install the connector package (same way as installed by end user) and runs the tests against the package?
  2. runs the tests against a source folder of the connector package without installing it as a package.

let's say we have a bug in packaging of the connector, that bug will be discovered if we do 1) but not if we do 2). I wonder which approach we are doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it's a combination of both. Since we are now using poetry, it guarantees that when we runpoetry install, the local code will be packaged and then installed equivalent to a user installing the published .tar.gz file with pip or similar.

I believe we're safe on this front.

#----------------------------------------------
- name: Run tests
run: poetry run pytest tests/
check-linting:
runs-on: ubuntu-latest
steps:
#----------------------------------------------
# check-out repo and set-up python
#----------------------------------------------
- name: Check out repository
uses: actions/checkout@v2
- name: Set up python
id: setup-python
uses: actions/setup-python@v2
with:
python-version: 3.7
#----------------------------------------------
# ----- install & configure poetry -----
#----------------------------------------------
- name: Install Poetry
uses: snok/install-poetry@v1
with:
virtualenvs-create: true
virtualenvs-in-project: true
installer-parallel: true

#----------------------------------------------
# load cached venv if cache exists
#----------------------------------------------
- name: Load cached venv
id: cached-poetry-dependencies
uses: actions/cache@v2
with:
path: .venv
key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ github.event.repository.name }}-${{ hashFiles('**/poetry.lock') }}
#----------------------------------------------
# install dependencies if cache does not exist
#----------------------------------------------
- name: Install dependencies
if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
run: poetry install --no-interaction --no-root
#----------------------------------------------
# install your root project, if required
#----------------------------------------------
- name: Install library
run: poetry install --no-interaction
#----------------------------------------------
# black the code
#----------------------------------------------
- name: Black
run: poetry run black --check src

check-types:
runs-on: ubuntu-latest
steps:
#----------------------------------------------
# check-out repo and set-up python
#----------------------------------------------
- name: Check out repository
uses: actions/checkout@v2
- name: Set up python
id: setup-python
uses: actions/setup-python@v2
with:
python-version: 3.7
#----------------------------------------------
# ----- install & configure poetry -----
#----------------------------------------------
- name: Install Poetry
uses: snok/install-poetry@v1
with:
virtualenvs-create: true
virtualenvs-in-project: true
installer-parallel: true

#----------------------------------------------
# load cached venv if cache exists
#----------------------------------------------
- name: Load cached venv
id: cached-poetry-dependencies
uses: actions/cache@v2
with:
path: .venv
key: venv-${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-${{ github.event.repository.name }}-${{ hashFiles('**/poetry.lock') }}
#----------------------------------------------
# install dependencies if cache does not exist
#----------------------------------------------
- name: Install dependencies
if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
run: poetry install --no-interaction --no-root
#----------------------------------------------
# install your root project, if required
#----------------------------------------------
- name: Install library
run: poetry install --no-interaction
#----------------------------------------------
# black the code
#----------------------------------------------
- name: Mypy
run: poetry run mypy src
1 change: 0 additions & 1 deletion python-37-slim-buster.dockerfile
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions test-container-with-reqs.dockerfile
View file
Open in desktop

This file was deleted.

5 changes: 0 additions & 5 deletions tests/docker-run-unit.sh
View file
Open in desktop

This file was deleted.

4 changes: 1 addition & 3 deletions tests/test_fetches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@

class FetchTests(unittest.TestCase):
"""
Unit tests for checking the fetch logic. See
qa/test/cmdexec/python/suites/simple_connection_test.py for integration tests that
interact with the server.
Unit tests for checking the fetch logic.
"""

@staticmethod
Expand Down
2 changes: 2 additions & 0 deletions tests/test_fetches_bench.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pyarrow as pa
import uuid
import time
import pytest

import databricks.sql.client as client
from databricks.sql.utils import ExecuteResponse, ArrowQueue
Expand Down Expand Up @@ -39,6 +40,7 @@ def make_dummy_result_set_from_initial_results(arrow_table):
for col_id in range(arrow_table.num_columns)]
return rs

@pytest.mark.skip(reason="Test has not been updated for latest connector API (June 2022)")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails because this test file was not updated as the connector API changed. Unclear if we should just remove this test completely or update it. I didn't find any notes on this subject.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@susodapop was there any breaking changes in the public API of the PySQL?

Did we do breaking change as part of open sourcing? or the breaking change was introduced before that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I recall the test is broken because we changed the API and didn't update this test accordingly. Since I'm not familiar with what this test is meant to do or why it wasn't updated I don't have further context.

def test_benchmark_fetchall(self):
print("preparing dummy arrow table")
arrow_table = FetchBenchmarkTests.make_arrow_table(10, 25000)
Expand Down
4 changes: 1 addition & 3 deletions tests/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@

class ClientTestSuite(unittest.TestCase):
"""
Unit tests for isolated client behaviour. See
qa/test/cmdexec/python/suites/simple_connection_test.py for integration tests that
interact with the server.
Unit tests for isolated client behaviour.
"""

PACKAGE_NAME = "databricks.sql"
Expand Down
6 changes: 0 additions & 6 deletions thrift_dbr_test/README.md
View file
Open in desktop

This file was deleted.

Empty file removed thrift_dbr_test/__init__.py
View file
Open in desktop
Empty file.
20 changes: 0 additions & 20 deletions thrift_dbr_test/thrift_basic_tests.py
View file
Open in desktop

This file was deleted.

34 changes: 0 additions & 34 deletions thrift_dbr_test/thrift_sqlquery_tests.py
View file
Open in desktop

This file was deleted.