Skip to content
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.

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

Replace pyodbc with pymssql #3435

Merged
merged 24 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
75a5197
Replace pyodbc with pymssql
ThomasLaPiana Jun 2, 2023
5cb32f6
feat: replace all instances of pyodbc with pymssql
ThomasLaPiana Jun 2, 2023
31102d5
docs: changelog
ThomasLaPiana Jun 2, 2023
0f53c9c
fix: docker health check
ThomasLaPiana Jun 2, 2023
1ca4066
feat: more progress on fixing the MSSQL tests
ThomasLaPiana Jun 2, 2023
d84c139
fix: update how mssql is seeded in tests
ThomasLaPiana Jun 2, 2023
cf92429
feat: update external database setup patterns
ThomasLaPiana Jun 2, 2023
147dc1b
fix: mssql server testing for ctl
ThomasLaPiana Jun 2, 2023
554eb6d
fix: run all mssql queries in a single connection
ThomasLaPiana Jun 3, 2023
5bc32a2
Merge branch 'main' into ThomasLaPiana-replace-pyodbc-pymssql
ThomasLaPiana Jun 3, 2023
eea9046
fix: remove the hardcoded driver from the mssql connector
ThomasLaPiana Jun 3, 2023
e3ff6c6
feat: add dependencies to allow for building on ARM
ThomasLaPiana Jun 5, 2023
56bedff
fix: connection tests
ThomasLaPiana Jun 5, 2023
c8fe8c6
fix: pylint
ThomasLaPiana Jun 5, 2023
ec1d5c3
fix: add other deps
ThomasLaPiana Jun 7, 2023
9b9adf5
Merge branch 'main' into ThomasLaPiana-replace-pyodbc-pymssql
ThomasLaPiana Jun 7, 2023
b30462d
fix: include freetds in backend build stage
SteveDMurphy Jun 15, 2023
7951f00
Merge branch 'main' into ThomasLaPiana-replace-pyodbc-pymssql
ThomasLaPiana Jun 15, 2023
057db90
fix: static checks
ThomasLaPiana Jun 15, 2023
eb30d53
Merge branch 'main' into ThomasLaPiana-replace-pyodbc-pymssql
ThomasLaPiana Jun 16, 2023
7fa3948
fix: merged changes, static checks
ThomasLaPiana Jun 16, 2023
6d09ed2
fix: import
ThomasLaPiana Jun 16, 2023
2fc1c34
fix: remove mention of pyodbc
ThomasLaPiana Jun 16, 2023
5f28b54
fix: move the new migration from `main` into the new migrations folder
ThomasLaPiana Jun 16, 2023
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
6 changes: 4 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ The types of changes are:
- `Fixed` for any bug fixes.
- `Security` in case of vulnerabilities.


## [Unreleased](https://github.com/ethyca/fides/compare/2.15.0...main)

### Added
Expand All @@ -24,6 +23,10 @@ The types of changes are:
- Support for acknowledge button for notice-only Privacy Notices and to disable toggling them off [#3546](https://github.com/ethyca/fides/pull/3546)
- HTML format for privacy request storage destinations [#3427](https://github.com/ethyca/fides/pull/3427)

### Changed

- Removed `pyodbc` in favor of `pymssql` for handling SQL Server connections [#3435](https://github.com/ethyca/fides/pull/3435)

### Fixed

- Fix race condition with consent modal link rendering [#3521](https://github.com/ethyca/fides/pull/3521)
Expand All @@ -36,7 +39,6 @@ The types of changes are:

- Optimize GitHub workflows used for docker image publishing [#3526](https://github.com/ethyca/fides/pull/3526)


## [2.15.0](https://github.com/ethyca/fides/compare/2.14.1...2.15.0)

### Added
Expand Down
59 changes: 28 additions & 31 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
# If you update this, also update `DEFAULT_PYTHON_VERSION` in the GitHub workflow files
ARG PYTHON_VERSION="3.10.11"

#########################
## Compile Python Deps ##
#########################
FROM python:${PYTHON_VERSION}-slim-bullseye as compile_image
ARG TARGETPLATFORM

# Install auxiliary software
RUN apt-get update && \
Expand All @@ -17,16 +15,33 @@ RUN apt-get update && \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*


# Install FreeTDS (used for PyMSSQL)
RUN apt-get update && \
apt-get install -y --no-install-recommends \
libssl-dev \
libffi-dev \
libxslt-dev \
libkrb5-dev \
unixodbc \
unixodbc-dev \
freetds-dev \
freetds-bin \
python-dev \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*

# Install Python Dependencies
COPY dev-requirements.txt .
RUN pip install --user -U pip --no-cache-dir install -r dev-requirements.txt

# Activate a Python venv
RUN python3 -m venv /opt/fides
ENV PATH="/opt/fides/bin:${PATH}"

# Install Python Dependencies
RUN pip --no-cache-dir --disable-pip-version-check install --upgrade pip setuptools wheel

COPY dangerous-requirements.txt .
RUN if [ $TARGETPLATFORM != linux/arm64 ] ; then pip install --no-cache-dir install -r dangerous-requirements.txt ; fi

COPY requirements.txt .
RUN pip install --no-cache-dir install -r requirements.txt

Expand All @@ -37,38 +52,20 @@ RUN pip install --no-cache-dir install -r dev-requirements.txt
## Backend Base ##
##################
FROM python:${PYTHON_VERSION}-slim-bullseye as backend
ARG TARGETPLATFORM

# Loads compiled requirements and adds the to the path
COPY --from=compile_image /opt/fides /opt/fides
ENV PATH=/opt/fides/bin:$PATH

# These are all required for MSSQL
RUN : \
&& apt-get update \
&& apt-get install \
-y --no-install-recommends \
apt-transport-https \
RUN apt-get update && \
apt-get install -y --no-install-recommends \
curl \
git \
gnupg \
unixodbc-dev \
freetds-dev \
freetds-bin \
python-dev \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*

# SQL Server (MS SQL)
# https://docs.microsoft.com/en-us/sql/connect/odbc/linux-mac/installing-the-microsoft-odbc-driver-for-sql-server?view=sql-server-ver15
RUN curl https://packages.microsoft.com/keys/microsoft.asc | apt-key add -
RUN curl https://packages.microsoft.com/config/debian/11/prod.list | tee /etc/apt/sources.list.d/msprod.list
ENV ACCEPT_EULA=y DEBIAN_FRONTEND=noninteractive
RUN if [ "$TARGETPLATFORM" != "linux/arm64" ] ; \
ThomasLaPiana marked this conversation as resolved.
Show resolved Hide resolved
then apt-get update \
&& apt-get install \
-y --no-install-recommends \
mssql-tools \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* ; \
fi
# Loads compiled requirements and adds the to the path
COPY --from=compile_image /opt/fides /opt/fides
ENV PATH=/opt/fides/bin:$PATH

# General Application Setup ##
COPY . /fides
Expand Down
5 changes: 0 additions & 5 deletions dangerous-requirements.txt

This file was deleted.

1 change: 0 additions & 1 deletion docs/fides/docs/development/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ The primary requirements for contributing to Fides are `Docker` and `Python`. Th
* CPUs: 4
* Memory:8GB
* Disk Limit: 200GB
2. There are known issues around connecting to MSSQL for Apple M1 users. M1 users that wish to install `pyodbc` locally, please reference the workaround [here](https://github.com/mkleehammer/pyodbc/issues/846).

Now that those are installed, the final step is to install the Python dev requirements for the Fides project. We recommend doing this in a virtual environment or using [pipx](https://pypa.github.io/pipx/), but specific details are outside the scope of this guide.

Expand Down
25 changes: 0 additions & 25 deletions noxfiles/docker_nox.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""Contains the nox sessions for docker-related tasks."""
import platform
from multiprocessing import Pool
from subprocess import run
from typing import Callable, Dict, List, Tuple
Expand All @@ -19,11 +18,6 @@
)
from git_nox import get_current_tag, recognized_tag

DOCKER_PLATFORM_MAP = {
"amd64": "linux/amd64",
"arm64": "linux/arm64",
"x86_64": "linux/amd64",
}
DOCKER_PLATFORMS = "linux/amd64,linux/arm64"


Expand Down Expand Up @@ -83,18 +77,6 @@ def get_current_image() -> str:
return f"{IMAGE}:{get_current_tag()}"


def get_platform(posargs: List[str]) -> str:
ThomasLaPiana marked this conversation as resolved.
Show resolved Hide resolved
"""
Calculate the CPU platform or get it from the
positional arguments.
"""
if "amd64" in posargs:
return DOCKER_PLATFORM_MAP["amd64"]
if "arm64" in posargs:
return DOCKER_PLATFORM_MAP["arm64"]
return DOCKER_PLATFORM_MAP[platform.machine().lower()]


@nox.session()
@nox.parametrize(
"image",
Expand All @@ -117,7 +99,6 @@ def build(session: nox.Session, image: str, machine_type: str = "") -> None:
prod = Build the fides webserver/CLI and tag it as the current application version.
test = Build the fides webserver/CLI the same as `prod`, but tag it as `local`.
"""
build_platform = get_platform(session.posargs)

# This check needs to be here so it has access to the session to throw an error
if image == "prod":
Expand Down Expand Up @@ -153,8 +134,6 @@ def build(session: nox.Session, image: str, machine_type: str = "") -> None:
"docker",
"build",
"--target=prod_pc",
"--platform",
build_platform,
"--tag",
privacy_center_image_tag,
".",
Expand All @@ -166,8 +145,6 @@ def build(session: nox.Session, image: str, machine_type: str = "") -> None:
"docker",
"build",
"clients/sample-app",
"--platform",
build_platform,
"--tag",
sample_app_image_tag,
external=True,
Expand All @@ -180,8 +157,6 @@ def build(session: nox.Session, image: str, machine_type: str = "") -> None:
"docker",
"build",
f"--target={target}",
"--platform",
build_platform,
"--tag",
tag(),
".",
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ pydantic<1.10.2
pydash==6.0.2
PyJWT==2.4.0
pymongo==3.13.0
pymssql==2.2.7
PyMySQL==1.0.2
python-jose[cryptography]==3.3.0
pyyaml>=5,<6
Expand Down
2 changes: 1 addition & 1 deletion scripts/mssql_discover.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import sqlalchemy

MASTER_MSSQL_URL = f"mssql+pyodbc://{USER}:{PASS}@{IP}:{PORT}/{DB}?driver=ODBC+Driver+17+for+SQL+Server"
MASTER_MSSQL_URL = f"mssql+pymssql://{USER}:{PASS}@{IP}:{PORT}/{DB}"


SUPPORTED_DATA_TYPES = set(
Expand Down
41 changes: 0 additions & 41 deletions setup.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import pathlib
from typing import List

from setuptools import find_packages, setup

Expand All @@ -14,45 +13,6 @@

install_requires = open("requirements.txt", encoding="utf-8").read().strip().split("\n")
dev_requires = open("dev-requirements.txt", encoding="utf-8").read().strip().split("\n")
dangerous_requires = (
open("dangerous-requirements.txt", encoding="utf-8").read().strip().split("\n")
)


def optional_requirements(
dependency_names: List[str], requires: List[str] = dangerous_requires
) -> List[str]:
"""
Matches the provided dependency names to lines in `optional-requirements.txt`,
and returns the full dependency string for each one.

Prevents the need to store version numbers in two places.
"""

requirements: List[str] = []

for dependency in dependency_names:
for optional_dependency in requires:
if optional_dependency.startswith(dependency):
requirements.append(optional_dependency)
break

if len(requirements) == len(dependency_names):
return requirements

raise ModuleNotFoundError


# Human-Readable Extras
# Versions are read from corresponding lines in `optional-requirements.txt`
extras = {
"mssql": optional_requirements(["pyodbc"], dangerous_requires),
}
dangerous_extras = ["mssql"] # These extras break on certain platforms
extras["all"] = sum(
[value for key, value in extras.items() if key not in dangerous_extras], []
)


###################
## Package Setup ##
Expand All @@ -75,7 +35,6 @@ def optional_requirements(
license="Apache License 2.0",
install_requires=install_requires,
dev_requires=dev_requires,
extras_require=extras,
classifiers=[
"License :: OSI Approved :: Apache Software License",
"Programming Language :: Python :: 3 :: Only",
Expand Down
2 changes: 1 addition & 1 deletion src/fides/api/api/v1/endpoints/connection_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
V1_URL_PREFIX,
)
from fides.api.common_exceptions import ClientUnsuccessfulException, ConnectionException
from fides.api.ctl.sql_models import Dataset as CtlDataset # type: ignore[attr-defined]
from fides.api.models.connectionconfig import (
ConnectionConfig,
ConnectionTestStatus,
Expand All @@ -49,6 +48,7 @@
TestStatusMessage,
)
from fides.api.service.connectors import get_connector
from fides.api.models.sql_models import Dataset as CtlDataset # type: ignore[attr-defined]
from fides.api.util.api_router import APIRouter
from fides.api.util.connection_util import (
patch_connection_configs,
Expand Down
4 changes: 3 additions & 1 deletion src/fides/api/api/v1/endpoints/dataset_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@
convert_dataset_to_graph,
to_graph_field,
)
from fides.api.models.sql_models import Dataset as CtlDataset # type: ignore[attr-defined]
from fides.api.models.sql_models import ( # type: ignore[attr-defined]
Dataset as CtlDataset,
)
from fides.api.oauth.utils import verify_oauth_client
from fides.api.schemas.api import BulkUpdateFailed
from fides.api.schemas.dataset import (
Expand Down
4 changes: 3 additions & 1 deletion src/fides/api/api/v1/endpoints/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
)
from fides.api.db.base import Base # type: ignore[attr-defined]
from fides.api.db.crud import get_resource, list_resource
from fides.api.models.sql_models import models_with_default_field # type: ignore[attr-defined]
from fides.api.models.sql_models import ( # type: ignore[attr-defined]
models_with_default_field,
)
from fides.api.util import errors
from fides.core.config import CONFIG

Expand Down
4 changes: 3 additions & 1 deletion src/fides/api/models/datasetconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
)
from fides.api.graph.data_type import parse_data_type_string
from fides.api.models.connectionconfig import ConnectionConfig, ConnectionType
from fides.api.models.sql_models import Dataset as CtlDataset # type: ignore[attr-defined]
from fides.api.models.sql_models import ( # type: ignore[attr-defined]
Dataset as CtlDataset,
)
from fides.api.util.saas_util import merge_datasets


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class MicrosoftSQLServerSchema(ConnectionConfigSecretsSchema):
"""Schema to validate the secrets needed to connect to a MS SQL Database

connection string takes the format:
mssql+pyodbc://[username]:[password]@[host]:[port]/[dbname]?driver=ODBC+Driver+17+for+SQL+Server
mssql+pymssql://[username]:[password]@[host]:[port]/[dbname]

"""

Expand Down
3 changes: 2 additions & 1 deletion src/fides/api/schemas/privacy_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
)
from fides.api.schemas.api import BulkResponse, BulkUpdateFailed
from fides.api.schemas.base_class import FidesSchema
from fides.api.schemas.policy import ActionType, PolicyResponse as PolicySchema
from fides.api.schemas.policy import ActionType
from fides.api.schemas.policy import PolicyResponse as PolicySchema
from fides.api.schemas.redis_cache import Identity
from fides.api.schemas.user import PrivacyRequestReviewer
from fides.api.util.encryption.aes_gcm_encryption_scheme import verify_encryption_key
Expand Down
5 changes: 2 additions & 3 deletions src/fides/api/service/connectors/sql_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,20 +497,19 @@ class MicrosoftSQLServerConnector(SQLConnector):
def build_uri(self) -> URL:
"""
Build URI of format
mssql+pyodbc://[username]:[password]@[host]:[port]/[dbname]?driver=ODBC+Driver+17+for+SQL+Server
mssql+pymssql://[username]:[password]@[host]:[port]/[dbname]
Returns URL obj, since SQLAlchemy's create_engine method accepts either a URL obj or a string
"""

config = self.secrets_schema(**self.configuration.secrets or {})

url = URL.create(
"mssql+pyodbc",
"mssql+pymssql",
username=config.username,
password=config.password,
host=config.host,
port=config.port,
database=config.dbname,
query={"driver": "ODBC Driver 17 for SQL Server"},
)

return url
Expand Down
4 changes: 3 additions & 1 deletion src/fides/api/util/data_category.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
from sqlalchemy.orm import Session

from fides.api import common_exceptions
from fides.api.models.sql_models import DataCategory as DataCategoryDbModel # type: ignore[attr-defined]
from fides.api.models.sql_models import ( # type: ignore[attr-defined]
DataCategory as DataCategoryDbModel,
)


def generate_fides_data_categories() -> Type[EnumType]:
Expand Down
Loading