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

Use different dev dependencies for different fedora versions #219

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ jobs:
cache: "pip"

- name: Install requirements
run: pip install -e '.[cassandra,dev,f${{ matrix.fedora-version }}]'
run: pip install -e '.[cassandra,f${{ matrix.fedora-version }}-dev,f${{ matrix.fedora-version }}]'

- name: Execute lints and tests
run: make tests
run: make test
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh ! Thanks for noticing that.


- id: upload-codecov
# Third-party action pinned to v2.1.0
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ repos:
- id: black

- repo: https://github.com/pycqa/flake8
rev: 6.0.0
rev: 7.0.0
hooks:
- id: flake8
files: \.py$
Expand Down
4 changes: 2 additions & 2 deletions astacus/coordinator/plugins/clickhouse/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
See LICENSE for details
"""
from astacus.coordinator.plugins.clickhouse.manifest import AccessEntity, Table
from collections.abc import Sequence
from typing import Callable, Hashable, TypeVar
from collections.abc import Hashable, Sequence
from typing import Callable, TypeVar

# noinspection PyCompatibility
import graphlib
Expand Down
27 changes: 26 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ f39 = [
"wcmatch == 8.4.1",
"zstandard == 0.21.0",
]
dev = [
f38-dev = [
Copy link
Contributor

Choose a reason for hiding this comment

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

pytest-cov f38 version is 4.0.0

# Needed by pre-commit to lint and test the project
"pre-commit>=3.7.0",
"anyio==3.5.0",
Expand All @@ -116,6 +116,31 @@ dev = [
"respx==0.20.1",
]

f39-dev = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the large number of missing dev deps in Fedora, and to keep the project somewhat normal for non-Fedora users, we should probably have a dev section with reasonably open ranges (the actual requirements), and then fX-dev sections will fully pinned versions of the packages that can be pinned to Fedora matched versions.

# Needed by pre-commit to lint and test the project
"pre-commit==3.7.0",
"anyio==3.7.0",
"pylint==3.0.4",
"pytest-asyncio==0.23.5",
"pytest-cov==4.0.0",
"pytest-mock==3.11.1",
"pytest-order",
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find this package in F38 or F39, this shouldn't be in sections named fX-dev.

"pytest-timeout==2.1.0",
"pytest-watch==4.2.0",
"pytest==7.3.2",
"mypy==1.8.0",
# Types for things that don't seem to have them
"types-botocore>=1.0.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find python3-types-botocore in F38 or F39, this shouldn't be in sections named fX-dev.

"types-PyYAML>=6.0.12.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

The version of python3-types-pyyaml is 6.0.1, both for F38 and F39.

"types-requests>=2.28.11.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

The version of python3-types-requests is 2.26.1, both for F38 and F39.

"types-tabulate>=0.9.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find python3-types-tabulate in F38 or F39, this shouldn't be in sections named fX-dev.

"types-ujson>=5.9.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find python3-types-ujson in F38 or F39, but thankfully we're not using ujson at all, so this can be removed.

"types-urllib3>=1.26.25.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

I couldn't find python3-types-urllib3 in F38 or F39, this shouldn't be in sections named fX-dev.

"coverage==7.2.7",
"freezegun==1.2.2",
"respx==0.20.2",
]

[project.urls]
"Homepage" = "https://github.com/Aiven-Open/astacus"
"Bug Tracker" = "https://github.com/Aiven-Open/astacus/issues"
Expand Down
14 changes: 2 additions & 12 deletions tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"""
from astacus.common.utils import build_netloc
from astacus.coordinator.plugins.zookeeper import KazooZooKeeperClient
from collections.abc import AsyncIterator, Iterator, Mapping, Sequence
from collections.abc import AsyncIterator, Mapping, Sequence
from pathlib import Path
from types import MappingProxyType

Expand All @@ -21,16 +21,6 @@
logger = logging.getLogger(__name__)


@pytest.fixture(scope="module", name="event_loop")
def fixture_event_loop() -> Iterator[asyncio.AbstractEventLoop]:
# This is the same as the original `event_loop` fixture from `pytest_asyncio`
# but with a module scope, re-declaring this fixture is their suggested way
# of locally increasing the scope of this fixture.
loop = asyncio.get_event_loop_policy().new_event_loop()
yield loop
loop.close()


async def get_command_path(name: str) -> Path | None:
process = await asyncio.create_subprocess_exec(
"which", name, stderr=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE
Expand Down Expand Up @@ -147,7 +137,7 @@ def fixture_ports() -> Ports:
return Ports()


@pytest.fixture(scope="module", name="zookeeper")
@pytest.fixture(name="zookeeper")
Copy link
Contributor

Choose a reason for hiding this comment

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

We really can't drop all module scopes fixtures, the tests are already quite slow and it's not viable to move everything to function scope.

(Don't look at the GHA test times, many tests don't run in that context.)

Thankfully we already have a solution used in core, I've added a PR to do the same in astacus: #220

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh good, I'd far prefer to move away from pytest-asyncio. I'll amend this PR to not include those changes, and I'll review the versions to make sure they include only the versions included there.

async def fixture_zookeeper(ports: Ports) -> AsyncIterator[Service]:
async with create_zookeeper(ports) as zookeeper:
yield zookeeper
Expand Down
10 changes: 5 additions & 5 deletions tests/integration/coordinator/plugins/clickhouse/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class ClickHouseServiceCluster(ServiceCluster):
ClickHouseCommand = Sequence[str | Path]


@pytest.fixture(scope="module", name="clickhouse_command")
@pytest.fixture(scope="function", name="clickhouse_command")
async def fixture_clickhouse_command(request: FixtureRequest) -> ClickHouseCommand:
clickhouse_path = request.config.getoption(CLICKHOUSE_PATH_OPTION)
if clickhouse_path is None:
Expand All @@ -91,7 +91,7 @@ async def fixture_clickhouse_command(request: FixtureRequest) -> ClickHouseComma
return get_clickhouse_command(clickhouse_path)


@pytest.fixture(scope="module", name="clickhouse_restore_command")
@pytest.fixture(scope="function", name="clickhouse_restore_command")
def fixture_clickhouse_restore_command(request: FixtureRequest, clickhouse_command: ClickHouseCommand) -> ClickHouseCommand:
clickhouse_restore_path = request.config.getoption(CLICKHOUSE_RESTORE_PATH_OPTION)
if clickhouse_restore_path is None:
Expand All @@ -103,7 +103,7 @@ def get_clickhouse_command(clickhouse_path: Path) -> ClickHouseCommand:
return [clickhouse_path] if clickhouse_path.name.endswith("-server") else [clickhouse_path, "server"]


@pytest.fixture(scope="module", name="clickhouse")
@pytest.fixture(scope="function", name="clickhouse")
async def fixture_clickhouse(ports: Ports, clickhouse_command: ClickHouseCommand) -> AsyncIterator[Service]:
async with create_clickhouse_service(ports, clickhouse_command) as service:
yield service
Expand Down Expand Up @@ -192,13 +192,13 @@ def bucket(self, *, bucket_name: str) -> Iterator[MinioBucket]:
s3_client.delete_bucket(Bucket=bucket_name) # type: ignore[attr-defined]


@pytest.fixture(scope="module", name="minio")
@pytest.fixture(scope="function", name="minio")
async def fixture_minio(ports: Ports) -> AsyncIterator[MinioService]:
async with create_minio_service(ports) as service:
yield service


@pytest.fixture(scope="module", name="minio_bucket")
@pytest.fixture(scope="function", name="minio_bucket")
async def fixture_minio_bucket(minio: MinioService) -> AsyncIterator[MinioBucket]:
with minio.bucket(bucket_name="clickhouse-bucket") as bucket:
yield bucket
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ async def restorable_cluster_manager(
yield storage_path


@pytest.fixture(scope="module", name="restorable_cluster")
@pytest.fixture(scope="function", name="restorable_cluster")
async def fixture_restorable_cluster(
ports: Ports,
clickhouse_command: ClickHouseCommand,
Expand Down Expand Up @@ -148,7 +148,7 @@ async def restored_cluster_manager(
yield clients


@pytest.fixture(scope="module", name="restored_cluster", params=[*get_restore_steps_names(), None])
@pytest.fixture(scope="function", name="restored_cluster", params=[*get_restore_steps_names(), None])
async def fixture_restored_cluster(
ports: Ports,
request: SubRequest,
Expand Down