-
-
Notifications
You must be signed in to change notification settings - Fork 262
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
fix: module params #967
fix: module params #967
Conversation
@VishnuSanal is attempting to deploy a commit to the sparckles Team on Vercel. A member of the Team first needs to authorize it. |
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.
it would suffice to pass argv values directly to the subprocess, since we are def doing the argument parsing inside Robyn#init
. the previous issue was changes getting overwritten from robyn#init (when doing Config() call in the ocnstructor.)
also, there were issues with #parse_known_args
returning a log_level=DEBUG
& the config expecting a log-level=DEBUG
. similarly for disable openapi
# Parsing the known and unknown arguments | ||
known_arguments, unknown_args = config.parser.parse_known_args() | ||
command = [sys.executable] | ||
|
||
# Convert known arguments to a list of strings suitable for subprocess.run | ||
known_args_list = [f"{key}={value}" for key, value in vars(known_arguments).items() if value is not None] | ||
|
||
# Combine the python executable, unknown arguments, and known arguments | ||
command = [sys.executable, *unknown_args, *known_args_list] |
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.
@sansyrox please enlighten me on why there was the need for this much jargon here...? ! 🤔 were there a reason to not pass them directly? am I missing somehtign?
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.
@VishnuSanal , good find. I don't see a reason why this should exist.
Let me do a final review
The failing tests are that of the web sockets, please try rerunning. |
CodSpeed Performance ReportMerging #967 will not alter performanceComparing Summary
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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! 👍 🔥
Great work @VishnuSanal ! |
Description
This PR fixes #966
Summary
This PR fixes params issue when running as a module
PR Checklist
Please ensure that:
Pre-Commit Instructions: