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

Enhancement: Replace Starlette Responses #626

Merged
merged 30 commits into from
Oct 23, 2022
Merged

Enhancement: Replace Starlette Responses #626

merged 30 commits into from
Oct 23, 2022

Conversation

Goldziher
Copy link
Contributor

@Goldziher Goldziher commented Oct 19, 2022

PR Checklist

  • Have you followed the guidelines in CONTRIBUTING.md?
  • Have you got 100% test coverage on new code?
  • Have you updated the prose documentation?
  • Have you updated the reference documentation?

This PR removes the usage of Starlette based Response objects by introducing our own version - with a compatible API.

Relates to #612

@Goldziher Goldziher force-pushed the replace-response branch 2 times, most recently from 563ce24 to e1d1b7a Compare October 20, 2022 15:47
@lgtm-com
Copy link

lgtm-com bot commented Oct 20, 2022

This pull request introduces 1 alert when merging 6c39d4e into ac18312 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 1 alert when merging 4b9463c into 2e6eb83 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 1 alert when merging 51cb655 into 2e6eb83 - view on LGTM.com

new alerts:

  • 1 for Unused import

@Goldziher Goldziher marked this pull request as ready for review October 21, 2022 18:42
@Goldziher
Copy link
Contributor Author

PR is still missing docs and some polishing touches, but it is substantially done and the tests are in place - so its a good time to start reviewing. Please do!

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 1 alert when merging cce7f81 into 2e6eb83 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 1 alert when merging 3205a60 into 2e6eb83 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 1 alert when merging 0478d38 into 4ee6f56 - view on LGTM.com

new alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request introduces 1 alert when merging 35b431c into 4ee6f56 - view on LGTM.com

new alerts:

  • 1 for Unused import

Copy link
Contributor

@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.

Haven't made all the way through yet!

docs/usage/16-templating/2-template-functions.md Outdated Show resolved Hide resolved
starlite/datastructures/response_containers.py Outdated Show resolved Hide resolved
starlite/response/base.py Outdated Show resolved Hide resolved
starlite/response/base.py Outdated Show resolved Hide resolved
@Goldziher Goldziher added Enhancement This is a new feature or request starlette-migration labels Oct 22, 2022
@infohash
Copy link
Contributor

So with this PR, instead of using starlette RedirectResponse in the middleware, I will be able to use RedirectResponse from starlite?

@Goldziher
Copy link
Contributor Author

So with this PR, instead of using starlette RedirectResponse in the middleware, I will be able to use RedirectResponse from starlite?

that is correct

@lgtm-com
Copy link

lgtm-com bot commented Oct 22, 2022

This pull request introduces 1 alert when merging 1711e21 into aef4b85 - view on LGTM.com

new alerts:

  • 1 for Unused import

Goldziher and others added 20 commits October 23, 2022 18:32
Co-authored-by: Peter Schutt <peter@topsport.com.au>
Co-authored-by: Peter Schutt <peter@topsport.com.au>
* removed starlette dependency for lifespan events

* updated tests

* fix async callable

* added cookie parser (#645)

* added cookie parser

* address review comments

* Replace get name (#646)

* add get_name helper

* added helper to handle enum

* update enum safeguard

* Removed starlette HTTPException as code dependency (#647)

* removed starlette HTTPException as code dependency

* address review comments
* updated background tasks

* updated test and docs

* address review comment
@lgtm-com
Copy link

lgtm-com bot commented Oct 23, 2022

This pull request introduces 1 alert when merging ba0fddb into e58dd11 - view on LGTM.com

new alerts:

  • 1 for Unused import

@Goldziher Goldziher merged commit 0949b14 into main Oct 23, 2022
@Goldziher Goldziher deleted the replace-response branch October 23, 2022 16:53
@lgtm-com
Copy link

lgtm-com bot commented Oct 23, 2022

This pull request introduces 1 alert when merging 79de83b into e58dd11 - view on LGTM.com

new alerts:

  • 1 for Unused import

@sonarcloud
Copy link

sonarcloud bot commented Oct 23, 2022

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 2 Code Smells

99.6% 99.6% Coverage
0.0% 0.0% Duplication

provinzkraut added a commit that referenced this pull request Oct 27, 2022
Fix lower-casing of header values. (#693)

A bug was introduced as part of #626, that would lower case header values
provinzkraut added a commit that referenced this pull request Oct 27, 2022
* Initial draft for server-side sessions and unified session-middleware interface

* Implement SessionMiddleware with backends, basic server-session backends. Integrate backends with existing tests

* Initial draft for server-side sessions and unified session-middleware interface

* Implement SessionMiddleware with backends, basic server-session backends. Integrate backends with existing tests

* Initial draft for server-side sessions and unified session-middleware interface

* Implement SessionMiddleware with backends, basic server-session backends. Integrate backends with existing tests

* v1.30.0

* docs: add mookrs as a contributor for doc (#637)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Minor code cleanup

* Improve SQLAlchemy backend, support sync/async

* Improved backwards compatibility, typing and tests

* Simplify session backend configuration

* Add expiry mechanism to FakeAsyncMemcached, improve typing

* Test all session backends

* Remove a merge artifact

* Remove identity-source support for now

* Add docstrings to session backends, improve typing

* Minor styling fixes

* Add reference docs for session middleware

* Fix type annotations

* Initial draft for server-side sessions and unified session-middleware interface

* Implement SessionMiddleware with backends, basic server-session backends. Integrate backends with existing tests

* Initial draft for server-side sessions and unified session-middleware interface

* Implement SessionMiddleware with backends, basic server-session backends. Integrate backends with existing tests

* Initial draft for server-side sessions and unified session-middleware interface

* Implement SessionMiddleware with backends, basic server-session backends. Integrate backends with existing tests

* v1.30.0

* docs: add mookrs as a contributor for doc (#637)

* docs: update README.md [skip ci]

* docs: update .all-contributorsrc [skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* Minor code cleanup

* Improved backwards compatibility, typing and tests

* Test all session backends

* Remove a merge artifact

* session docs draft

* Disable MD052 in markdownlint

MD052 messes with mkdocs material's code highlighting

* Add aiomcache and redis types to pylint dependencies

* Remove `backend` property from BaseBackendConfig.

This proved difficult to type correctly, and I was hitting python/mypy#12113 (or some variety), so I removed it for now

* Add / update reference and usage documentation + some docstrings for session middleware

* Remove unused imports

* Move session backend config to the top

* Some minor documentation adjustments

* Improve docstrings

Add missing args and returns, add missing docstrings

* Rename SessionBackend > BaseSessionBackend

* Add _sync suffix to private synchronous methods

* Move all type variables to top of the file

* Use explicit "import from" consistently

* Make FileStorageMetadataWrapper a NamedTuple

* Add `make_filename` option to FileBackendConfig

* Move fake_memcached.FakeAsyncMemcached to tests/mocks.py

* Implement MemoryBackend using SimpleCacheBackend

* Add __slots__ for session middleware

* Add support for all session backends to TestClient

* Update starlite/middleware/session/sqlalchemy_backend.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/testing/test_client/client.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/middleware/session/sqlalchemy_backend.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/middleware/session/memcached_backend.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/middleware/session/sqlalchemy_backend.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/middleware/session/cookie_backend.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/middleware/session/base.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/testing/test_client/client.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Update starlite/testing/test_client/client.py

Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>

* Implement support for session backends in TestClient

* Make sqlalchemy session examples "run as-is"

* Fix lower-casing of header values. (#693)

A bug was introduced as part of #626, that would lower case header values

* Remove types-redis

* Add middleware bypassing to SessionMiddleware

Co-authored-by: Peter Schutt <peter.github@proton.me>
Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Na'aman Hirschfeld <nhirschfeld@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement This is a new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants