Skip to content

Commit

Permalink
Install psycopg2 based on platform (#60)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikealfare
mikealfare authored Apr 12, 2024
1 parent 461616f commit 32c8547
Show file tree
Hide file tree
Showing 13 changed files with 47 additions and 3,424 deletions.
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240412-153154.yaml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Determine `psycopg2` based on `platform_system` (Linux or other)
time: 2024-04-12T15:31:54.861201-04:00
custom:
Author: mikealfare
Issue: "60"
30 changes: 28 additions & 2 deletions .github/workflows/integration-tests.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ concurrency:
group: ${{ github.workflow }}-${{ github.event_name }}-${{ contains(github.event_name, 'pull_request') && github.event.pull_request.head.ref || github.sha }}
cancel-in-progress: true

defaults:
run:
shell: bash

jobs:
integration:
name: Integration Tests
Expand Down Expand Up @@ -68,14 +72,12 @@ jobs:

- name: Update Adapters and Core branches
if: ${{ github.event_name == 'workflow_call' || github.event_name == 'workflow_dispatch'}}
shell: bash
run: |
./.github/scripts/update_dev_packages.sh \
${{ inputs.dbt_adapters_branch }} \
${{ inputs.core_branch }}
- name: Setup postgres
shell: bash
run: psql -f ./scripts/setup_test_database.sql
env:
PGHOST: localhost
Expand Down Expand Up @@ -106,3 +108,27 @@ jobs:
source-file: "results.csv"
file-name: "integration_results"
python-version: ${{ matrix.python-version }}

psycopg2-check:
name: "Test psycopg2 build version"
runs-on: ${{ matrix.scenario.platform }}
strategy:
fail-fast: false
matrix:
scenario:
- {platform: ubuntu-latest, psycopg2-name: psycopg2}
- {platform: macos-latest, psycopg2-name: psycopg2-binary}
steps:
- name: "Check out repository"
uses: actions/checkout@v4

- name: "Test psycopg2 name"
run: |
python -m pip install .
PSYCOPG2_PIP_ENTRY=$(pip list | grep "psycopg2 " || pip list | grep psycopg2-binary)
echo $PSYCOPG2_PIP_ENTRY
PSYCOPG2_NAME="${PSYCOPG2_PIP_ENTRY%% *}"
echo $PSYCOPG2_NAME
if [[ "${PSYCOPG2_NAME}" != "${{ matrix.scenario.psycopg2-name }}" ]]; then
exit 1
fi
56 changes: 0 additions & 56 deletions hatch_build.py
View file

This file was deleted.

17 changes: 13 additions & 4 deletions pyproject.toml
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[project]
dynamic = ["version", "dependencies"]
dynamic = ["version"]
name = "dbt-postgres"
description = "The set of adapter protocols and base functionality that supports integration with dbt-core"
readme = "README.md"
Expand All @@ -22,6 +22,18 @@ classifiers = [
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: 3.11",
]
dependencies = [
# install `psycopg2` on linux (assumed production)
'psycopg2>=2.9,<3.0; platform_system == "Linux"',

This comment has been minimized.

Sign in to view

Copy link
@andy-clapson

andy-clapson May 14, 2024

@mikealfare assuming platform = Linux to be prod (and not-that to be dev) is a big assumption.
Why not let this be confirmed? Keeping DBS_PSYCOPG2_NAME around feels like a better solution than this.

This comment has been minimized.

Sign in to view

Copy link
@mikealfare

mikealfare May 15, 2024

Author Contributor

Agreed, it's certainly not perfect, but it felt like the best option of the options we had.

The issue is that we can't run something like pip install psycopg2 --no-binary because psycopg2 hosts the binary package as a completely separate package on PyPI. So we effectively need to swap dependencies out at build time. While the environment variable trick does work, it requires setup.py and some pretty non-standard logic in setup.py to boot. And with python 3.12 support coming up soon, this was not an option.

We thought about the various use cases that would require the standard psycopg2 install (e.g. a production installation of dbt-postgres) versus the use cases where psycopg2-binary would be preferred (e.g. developing on dbt-postgres or running dbt-postgres locally for small projects). This distinction leaves some gaps. Most notably, those gaps include folks who develop on linux and folks installing dbt-postgres in CI runs. However, this compromise led to the least drawbacks for production installations while also providing for a workaround to cover these gaps. Any non-linux user could simply uninstall psycopg2-binary and install psycopg2 if they really needed the performance gains. And any linux user could install the OS dependencies (e.g. sudo apt-get install libpq-dev) to support the standard build.

Perhaps the framing of the assumption was more harsh or terse than it could have been. Hopefully this provides some more context as to why we made the decision we made.

This comment has been minimized.

Sign in to view

Copy link
@andy-clapson

andy-clapson May 15, 2024

I see the issue from your side, certainlty. And yep, indeed we did see the issue in our CI pipelines.

Was a major release version considered?
# install `psycopg2-binary` on macos/windows (assumed development)
'psycopg2-binary>=2.9,<3.0; platform_system != "Linux"',
"dbt-adapters>=0.1.0a1,<2.0",
# add dbt-core to ensure backwards compatibility of installation, this is not a functional dependency
"dbt-core>=1.8.0a1",
# installed via dbt-adapters but used directly
"dbt-common>=0.1.0a1,<2.0",
"agate>=1.0,<2.0",
]

[project.urls]
Homepage = "https://github.com/dbt-labs/dbt-postgres"
Expand All @@ -43,9 +55,6 @@ packages = ["dbt"]
[tool.hatch.version]
path = "dbt/adapters/postgres/__version__.py"

[tool.hatch.build.hooks.custom]
path = "./hatch_build.py"

[tool.hatch.envs.default]
dependencies = [
"dbt-adapters @ git+https://github.com/dbt-labs/dbt-adapters.git",
Expand Down
Loading

0 comments on commit 32c8547

Please sign in to comment.