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.
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
Config from object string #1436
Config from object string #1436
Changes from 9 commits
e978121
0b64fe6
19b304b
a33ebba
375ebd3
bf029c1
f2a55d0
b36dc22
62420e0
eacf78b
bee7cfa
7347306
b534df2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I might be overthinking this, but would it be a good idea to support relative string paths?
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.
well, i use this function in Flask for configure app using config_from_object and is very good.
Because is common in application has config classes.
example:
In this case the user instanciate Sanic class and configure according from environment pass the string path of the class get from environment variable for example.
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.
Oh I am totally with you on this feature. I was just curious if it might be possible to support relative path as well.
i.e. Instead of
module1.module2.Class
, if you are already insidemodule1
, thenmodule2.Class
should suffice.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.
@harshanarayana I think that the
importlib
searches the entire loaded packages, so I guess this already works out of the box - I might be wrong, testing still needed.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.
@vltr It works, but the method invocation might need to be tweaked a bit to provide another helper argument.
import_module(name, package=None)
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.
@harshanarayana oh, ok, got it. Some tweaking may be needed then ...
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.
@jotagesales looks like this is still outstanding - can you make changes so we can get this in soon?
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.
@sjsadowski ready ;)
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.
Can you please rename this class from
Config
to something else? This line is overriding the import done byfrom sanic.config import Config
This is causing tests to fail.
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.
@harshanarayana i renamed the class, thank.
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.
Renaming this class seems to be the only thing holding it up.