Azure / azure-functions-durable-python

Python library for using the Durable Functions bindings.
MIT License
134 stars 51 forks source link

Parameter names using snake_case cause failure #498

Open Goldziher opened 2 months ago

Goldziher commented 2 months ago

🐛 Describe the bug When a parameter is given in snake_case, this causes a failure of the function framework to run:

@blueprint.activity_trigger(input_name="file_id")
async def handle_save_blob_data(
    file_id: str,
) -> None:
   ...

This fails, forcing us to use camelCase:

@blueprint.activity_trigger(input_name="fileId")
async def handle_save_blob_data(
    fileId: str,
) -> None:
   ...

Which works, but is not pythonic at all.

🤔 Expected behavior Snake case should be supported. This is pretty silly.

Steps to reproduce See above.

nytian commented 2 months ago

Hi, @Goldziher There is a restriction in binding pattern at Azure Function host which can be found here . In general, only alphanumeric character can be accepted by host. Thus, a binding name contains _ is not allowed. Thanks for bringing up this and I will open a PR later to update the parameter description for better understandings.

Goldziher commented 2 months ago

Hi, @Goldziher There is a restriction in binding pattern at Azure Function host which can be found here . In general, only alphanumeric character can be accepted by host. Thus, a binding name contains _ is not allowed. Thanks for bringing up this and I will open a PR later to update the parameter description for better understandings.

Thanks, but can't it be worked around? I would imagine it's easy to convert camel case to snake case and vice versa

nytian commented 2 months ago

@davidmrdavid David, can you help confirm if we can enable it? I guess the answer is no since we don't have our own(python-specific) bindings settings or check. Thanks!

davidmrdavid commented 1 month ago

@Goldziher: yes, this is a bit of a pain point, naturally Python programmers will create snake_case declarations and those tend to fail unexpectedly. This is one of those issues that seems easy to fix at a surface level, but requires coordination across several teams and their respective release cycles (the fix here is probably at the 'WebJobs' layer, which is really deep in the .NET piece of Azure Functions) and ultimately, due to there being a workaround, is hard to prioritize.

That said, @nytian - ideally we would throw an error if we detect an _ in the variable names passed in as strings to our decorators. That would be better than failing silently. If that validation doesn't exist today, we should add it.

Goldziher commented 1 month ago

@Goldziher: yes, this is a bit of a pain point, naturally Python programmers will create snake_case declarations and those tend to fail unexpectedly. This is one of those issues that seems easy to fix at a surface level, but requires coordination across several teams and their respective release cycles (the fix here is probably at the 'WebJobs' layer, which is really deep in the .NET piece of Azure Functions) and ultimately, due to there being a workaround, is hard to prioritize.

That said, @nytian - ideally we would throw an error if we detect an _ in the variable names passed in as strings to our decorators. That would be better than failing silently. If that validation doesn't exist today, we should add it.

Well, I can't argue with your internal processes but this is rather annoying.

It might be to much effort for you guys, but you could simply remap snake case to camel case and record this on the decorator instance itself. It could be part of the logic of the abstract class and thus extensible.

davidmrdavid commented 1 month ago

@Goldziher: possibly, I'd need to sync with the Azure Functions Python team as I'd want to remain consistent with their experience. @vrdmr: is this something y'all have considered?