Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Require dependencies based on plugin config #213

Merged
merged 31 commits into from
Jan 19, 2023
Merged

Conversation

gazorby
Copy link
Collaborator

@gazorby gazorby commented Jan 8, 2023

Fixes #211

@gazorby gazorby requested a review from peterschutt January 8, 2023 16:31
@gazorby gazorby self-assigned this Jan 8, 2023
Copy link
Member

@peterschutt peterschutt left a comment

Choose a reason for hiding this comment

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

Thanks @gazorby - happy to discuss any of the points I've raised.

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
src/starlite_saqlalchemy/init_plugin.py Outdated Show resolved Hide resolved
src/starlite_saqlalchemy/scripts.py Outdated Show resolved Hide resolved
src/starlite_saqlalchemy/service.py Outdated Show resolved Hide resolved
tests/unit/conftest.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
tests/unit/test_init_plugin_no_extras.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@gazorby gazorby force-pushed the build/extra-dependencies branch from ebb6f47 to 228049d Compare January 12, 2023 07:35
@peterschutt
Copy link
Member

Thanks for rebasing this @gazorby :) I hope it wasn't too much of a pain.

What do you think about doing SQLAlchemy as well?

@gazorby
Copy link
Collaborator Author

gazorby commented Jan 12, 2023

This allows me to keep up with the latest changes ;)

Was thinking about it as well, but we need to fix #174 then

@gazorby
Copy link
Collaborator Author

gazorby commented Jan 16, 2023

Do you want this one in #245 too @peterschutt?

@peterschutt
Copy link
Member

Do you want this one in #245 too @peterschutt?

No rush - we can make a new release any time. Can I do anything to help on this one?

@gazorby
Copy link
Collaborator Author

gazorby commented Jan 16, 2023

Ok, so we just miss sqla added to extras then. Are you happy with the noextras tox env or do you want a more thorough import logic to avoid specifying test?

@gazorby gazorby force-pushed the build/extra-dependencies branch from 386f9d6 to 3f10824 Compare January 16, 2023 17:06
@gazorby gazorby force-pushed the build/extra-dependencies branch from 5d81b27 to 2b5d9c4 Compare January 17, 2023 07:35
@gazorby
Copy link
Collaborator Author

gazorby commented Jan 17, 2023

Hey @peterschutt, sqlalchemy has been added to optional dependencies and the noextras env is now passing the full test suite. Feel free to update wording in the readme.

@gazorby gazorby requested a review from peterschutt January 17, 2023 19:10
@peterschutt
Copy link
Member

Hey @peterschutt, sqlalchemy has been added to optional dependencies and the noextras env is now passing the full test suite. Feel free to update wording in the readme.

This is a stack of work! Thanks so much for this!

pyproject.toml Outdated Show resolved Hide resolved
Copy link
Member

@peterschutt peterschutt left a comment

Choose a reason for hiding this comment

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

Hey @gazorby - thanks again, a lot of these comments are for my own notes - I'm going to start working through some updates for them.

pyproject.toml Show resolved Hide resolved
requirements.dev.txt Outdated Show resolved Hide resolved
src/pytest_starlite_saqlalchemy/plugin.py Show resolved Hide resolved
src/starlite_saqlalchemy/__init__.py Outdated Show resolved Hide resolved
src/starlite_saqlalchemy/repository/__init__.py Outdated Show resolved Hide resolved
tests/unit/test_init_plugin.py Outdated Show resolved Hide resolved
tests/unit/test_log.py Outdated Show resolved Hide resolved
tests/unit/test_service.py Outdated Show resolved Hide resolved
tests/unit/test_testing.py Outdated Show resolved Hide resolved
tests/utils/app.py Outdated Show resolved Hide resolved
Default values for `PluginConfig.do_cache` et al. are based on the
`IS_XXX_INSTALLED` constants.

We use `default_factory` so we can patch the module values in tests.

Use `@validator` to test for missing dependency when gates are
explicitly set.
@peterschutt peterschutt force-pushed the build/extra-dependencies branch from b7aaee6 to 4ad9cb3 Compare January 18, 2023 11:15
tests/unit/conftest.py Outdated Show resolved Hide resolved
Import from within the test/fixture where appropriate.
@peterschutt peterschutt force-pushed the build/extra-dependencies branch from 54f790c to da3e4e6 Compare January 18, 2023 11:42
@peterschutt peterschutt changed the base branch from main to 0.29 January 18, 2023 12:06
* refactor: organise tests according to dependencies

- organises tests according to their dependencies.
- removes unnecessary autouse fixtures
- removes gated imports at the module level
- uses `--ignore` to exclude redis/sqlalchemy/sentry tests from no-extras tests

* refactor: use `test_ignore_glob` pytest global

This lets us skip test collection for a whole sub-package of tests if
a dependency is unavailable.

* refactor: log tests can run without sqlalchemy.

This was only necessary due to the dependency on
`tests.utils.controllers` in the `http_scope` fixture, which has now
been removed.

* refactor: don't need to skipif saq log tests

As the log tests depend on `job` fixture which will automatically skip
if SAQ not available.

* refactor: remaining fixtures/tests requiring sqlalchemy moved into sqlalchemy directory.
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@peterschutt peterschutt merged commit 3e48bbf into 0.29 Jan 19, 2023
@peterschutt peterschutt deleted the build/extra-dependencies branch January 19, 2023 00:43
@peterschutt
Copy link
Member

Thanks heaps @gazorby !

peterschutt added a commit that referenced this pull request Jan 20, 2023
* ♻️ refactor(deps): require dependencies based on plugin config

* 🐛 fix: tox config

* ♻️ refactor(tox): append coverage in testenv:noextras

* ♻️ refactor(tox): don't run coverage in parallel mode in testenv:noextras

* 🐛 fix: full coverage

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ♻️ refactor: apply changes from review

* 🐛 fix: linters

* ♻️ refactor: full coverage

* ♻️ refactor: update pytest plugin fixtures

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* ✅ test: fix tests

* ♻️ refactor: add sqlalchemy to optional dependencies

* ♻️ refactor: silent flake8

* ✅ test(tox): make noextras passing the full test suite

* ♻️ refactor: silent pyright

* 📝 docs: update readme

* 📝 docs: update readme

* refactor: requirements/tox/etc

* style: rollback style changes

The bigger the PR, the better it is if we keep the changes to bare
minimum.

* refactor: run pytest plugin tests in sub-process.

This prevents the need to reload any modules, albeit it is a bit slower.

* refactor: no top level conditional imports.

* refactor: makes lifespan check imports and logic conditional

* refactor: no implicit imports of modules with optional functionality

* refactor: move any worker related stuff out of service module

* refactor: default_factory and validator for dependency-based gates

Default values for `PluginConfig.do_cache` et al. are based on the
`IS_XXX_INSTALLED` constants.

We use `default_factory` so we can patch the module values in tests.

Use `@validator` to test for missing dependency when gates are
explicitly set.

* refactor: remove more module scoped conditional imports.

Import from within the test/fixture where appropriate.

* refactor: organise tests according to dependencies (#257)

* refactor: organise tests according to dependencies

- organises tests according to their dependencies.
- removes unnecessary autouse fixtures
- removes gated imports at the module level
- uses `--ignore` to exclude redis/sqlalchemy/sentry tests from no-extras tests

* refactor: use `test_ignore_glob` pytest global

This lets us skip test collection for a whole sub-package of tests if
a dependency is unavailable.

* refactor: log tests can run without sqlalchemy.

This was only necessary due to the dependency on
`tests.utils.controllers` in the `http_scope` fixture, which has now
been removed.

* refactor: don't need to skipif saq log tests

As the log tests depend on `job` fixture which will automatically skip
if SAQ not available.

* refactor: remaining fixtures/tests requiring sqlalchemy moved into sqlalchemy directory.

* refactor: continue splitting tests up by required dependency.

Health checks are up.

* refactor: add method for setting lifecycle handlers.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Peter Schutt <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dependency extras
2 participants