-
-
Notifications
You must be signed in to change notification settings - Fork 334
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
Feature: Implements on_app_init
hook
#499
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peterschutt
force-pushed
the
on-app-config-hook
branch
from
September 20, 2022 22:45
c2ae06c
to
ed8c247
Compare
This pull request introduces 1 alert when merging ed8c247 into c9387f8 - view on LGTM.com new alerts:
|
peterschutt
force-pushed
the
on-app-config-hook
branch
from
September 20, 2022 22:57
ed8c247
to
70fd16f
Compare
peterschutt
commented
Sep 21, 2022
Registered hooks must receive and return an instance of `AppConfig` object. The `AppConfig` object is first instantiated from parameters passed to `Starlite.__init__()`, and then passed to each of the registered hook handlers. The resultant `AppConfig` instance is used to instantiate the application. Removes the use of `pydantic.validate_arguments()` on `Starlite.__init__()` as the instantiation of the `AppConfig` object does the same thing. Test to ensure that the parameters of the `Starlite` constructor are matched by properties on `AppConfig`. Test that ensures all properties on the `AppConfig` object are accessed within the `Starlite` constructor.
peterschutt
force-pushed
the
on-app-config-hook
branch
from
September 21, 2022 10:25
6be7da9
to
87d8024
Compare
cofin
approved these changes
Sep 21, 2022
Goldziher
reviewed
Sep 21, 2022
Goldziher
approved these changes
Sep 21, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Please update the docs before merging, and also add attribute comments in the AppConfig class.
peterschutt
changed the title
RFC: Implements
Feature: Implements Sep 22, 2022
on_app_config
hooks.on_app_init
hook
# Conflicts: # starlite/app.py
Any parameter that we did something like `thing or []` with inside `Starlite.__init__()` and `Router.__init__()`, I've moved the `thing or []` bit to the `AppConfig` instantiation point. This makes it nicer to use the `AppConfig` object externally, otherwise need to to the `if None` dance for every collection property on access.
peterschutt
commented
Sep 23, 2022
peterschutt
commented
Sep 23, 2022
Co-authored-by: Peter Schutt <peter@topsport.com.au>
Goldziher
approved these changes
Sep 23, 2022
Kudos, SonarCloud Quality Gate passed! |
Alex-CodeLab
pushed a commit
to Alex-CodeLab/starlite
that referenced
this pull request
Sep 30, 2022
Implements `on_app_config` hooks. Registered hooks must receive and return an instance of `AppConfig` object. The `AppConfig` object is first instantiated from parameters passed to `Starlite.__init__()`, and then passed to each of the registered hook handlers. The resultant `AppConfig` instance is used to instantiate the application. Removes the use of `pydantic.validate_arguments()` on `Starlite.__init__()` as the instantiation of the `AppConfig` object does the same thing. Test to ensure that the parameters of the `Starlite` constructor are matched by properties on `AppConfig`. Test that ensures all properties on the `AppConfig` object are accessed within the `Starlite` constructor.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
on_app_init
callbacksIssue with the current
PluginProtocol.on_app_init()
system.Plugin.on_app_init()
callback occurs before route handler registration, so plugins do nothave the opportunity to receive the handlers passed to the application before they are applied to
the app.
Therefore, if a plugin wants to manage or modify these attributes, the plugins need to perform this
step of pre-processing, or at least understand it, before they can operate on these configuraion
attributes. E.g.,
as_async_callable_list()
.configuration/attribute changes, or changes to wrapper objects in later releases may impact the way
plugins need to work to operate on certain configuration values.
PluginProtocol.on_app_init()
becoming popular and making a rod for our backs in terms of internal refactoring.Proposed solution
AppConfig
. We already use thevalidate_arguments()
decorator, that constructs a signaturemodel to validate arguments passed to the app constructor, so this would just be replacing that
validation with an explicit model.
AppConfig
object.Starlite
,any handlers registered for
on_app_config
callback receive theAppConfig
instance and mustreturn it.
AppConfig
object in the order that they are passed tothe
Starlite
constructor and must define their own behavior and recommendations for where theplugin should be inserted into the callback list.
the
Starlite
interface.Proposed implementation
The implementation is totally backward compatible.
Registered
on_app_config
hook handlers must receive and return an instance ofAppConfig
.The
AppConfig
object is first instantiated from parameters passed toStarlite.__init__()
, and then passed to each of the registered hook handlers.The resultant
AppConfig
instance is used to instantiate the application.Removes the use of
pydantic.validate_arguments()
onStarlite.__init__()
as the instantiation of theAppConfig
object does the same thing.There is a test to ensure that the parameters of the
Starlite
constructor are matched by properties onAppConfig
.There is a test that ensures all properties on the
AppConfig
object are accessed within theStarlite
constructor.PR Checklist
CONTRIBUTING.md
?