-
Notifications
You must be signed in to change notification settings - Fork 245
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
Enable developers to inject non-bolt arguments to listener function args #709
Comments
Seems like a reasonable request to remove that part of the code. @seratch is the kwargs scrubbing code that @cooperbenson-qz linked to necessary to keep? |
I'm happy to make a PR based on whatever determination you come to 😁 |
I agree that we can stop explicitly setting None to unknown args there, but I still would like to maintain the default behavior to print the warning logs. The log intends to make the dev experience more friendly by informing misspellings/typos of argument names. Perhaps, adding a new option to disable the check to |
I like the idea of needing to specify an option to disable the "not a valid argument" warning message in situations where the framework is being used in an "expert" manner. (I don't know if there are other similar log messages in the code where it might be nice to suppress them in less-typical use cases. If there are, naming the option something "generic" might be worthwhile so the option can be used in multiple places, instead of adding one for each warning whenever a situation like this comes up.) |
Perhaps instead of using the logger to output the warning message, it might make sense to use warnings since that gives the user a lot of options for filtering and surpassing warnings. |
…stener function args
@eddyg @cooperbenson-qz Thanks a lot for sharing your great insights. Indeed, using warnings for this purpose makes sense. Please feel free to add comments to #712 if you have any. |
Sorry. After working on #712 a bit more, I've concluded my solution can't work at all and I cannot think of any elegant solutions for it. I myself am not going to use my time for it anymore. If someone can enhance bolt-python to support the use cases mentioned here without breaking any existing code, we are happy to review the changes. Until then, a workaround that I suggest is: @inject
def initialize_listeners(
app: AsyncApp,
# Inject your components here
version: str = Provide[Container.config.VERSION],
):
@app.command('/ping')
async def handle_ping(ack, respond):
await ack()
await respond({
"blocks": [
SectionBlock(text=MarkdownTextObject(text='Pong!')),
DividerBlock(),
ContextBlock(elements=[
MarkdownTextObject(text=f'*Host:* {platform.node()}'),
MarkdownTextObject(text=f'*Version:* {version}')
])
]
})
app = AsyncApp()
initialize_listeners(app)
if __name__ == "__main__":
app.start() |
I'm using dependency_injector to provide automatic dependency injection for my application, and I'm encountering an issue with Bolt overriding injected parameters with
None
since it doesn't recognize them.Reproducible in:
The
slack_bolt
versionPython runtime version
OS info
Steps to reproduce:
(Share the commands to run, source code, and project settings (e.g., setup.py))
Expected result:
Bolt doesn't touch the
version
kwarg, but still injects theack
andrespond
kwargs.Actual result:
version
ends up being set toNone
and Bolt logs the following warning:version is not a valid argument
.This is caused by this line.
Requirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
The text was updated successfully, but these errors were encountered: