Skip to content
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

Enabled implicit output for generic bindings #1391

Merged
merged 26 commits into from
Mar 6, 2024

Conversation

hallvictoria
Copy link
Contributor

@hallvictoria hallvictoria commented Jan 5, 2024

Description

Enables implicit output for generic types. This addresses an ask from the Logic Apps team for generic type bindings to have implicit return.

In implementing this, two main errors occurred:

  1. Durable functions started failing with "FunctionLoadError: cannot load the DurableFunctionsHttpStart1 function: Python return annotation "HttpResponse" does not match binding type "durableClient""
  • Cause: durableClient is a generic type and implicit output should be False
  • Fix: durableClient has its own converter class in the python sdk, so it is no longer a generic type. We h;ave a special case for this type in the worker to maintain backwards compatibility
  1. get_eventhub_batch_triggered failed with "TypeError: unable to encode outgoing TypedData: unsupported type "<class 'azure_functions_worker.bindings.generic.GenericBinding'>" for Python type "HttpResponse""
  • Cause: functions now can have both explicit and implicit output set to true. Previously, we set the function's return type to be the type of the binding that either is called $return or has implicit output set to true. In the case where there's a generic binding (implicit output = true) and $return in the same function, return type gets overwritten, and it depends on the order in which the bindings are processed to see what the final return type is set to.
  • Fix: prioritize the return type associated with $return, if that is present.
    • Case 1: $return processed before generic type
      -The boolean will be set to true and prevent the return type from later being overwritten
    • Case 2: generic type processed before $return
      -The $return return type will overwrite the implicit output binding return type
    • Case 3: no $return
      -The return type will be set to that of the implicit output binding

Fixes #


PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • If applicable, the PR references the bug/issue that it fixes in the description.
  • New Unit tests were added for the changes made and CI is passing.

Quality of Code and Contribution Guidelines

Copy link

codecov bot commented Jan 5, 2024

Codecov Report

Attention: Patch coverage is 93.33333% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 85.61%. Comparing base (2e91fa1) to head (466ee73).

Files Patch % Lines
azure_functions_worker/bindings/meta.py 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1391      +/-   ##
==========================================
+ Coverage   85.19%   85.61%   +0.42%     
==========================================
  Files          35       35              
  Lines        1965     1974       +9     
  Branches      370      374       +4     
==========================================
+ Hits         1674     1690      +16     
+ Misses        217      213       -4     
+ Partials       74       71       -3     
Flag Coverage Δ
unittests 85.56% <93.33%> (+0.42%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@YunchuWang
Copy link
Contributor

/profile

@hallvictoria hallvictoria marked this pull request as ready for review February 6, 2024 18:08
Copy link

@nytian nytian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

@hallvictoria hallvictoria merged commit c7ab3e9 into dev Mar 6, 2024
50 of 51 checks passed
@hallvictoria hallvictoria deleted the hallvictoria/enable_generic_implicit_output branch March 6, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants