Skip to content

Conversation

alexrashed
Copy link
Member

Motivation

The latest botocore specs contain a new recursive type definition (RetrievalFilter -> RetrievalFilterList -> RetrievalFilter).
These new types have been added to botocore with boto/botocore@667d4c6, which landed in version 1.35.72.
This caused an issue with the ASF scaffolding because of an issue in the code generation for recursive types which also contain a Python keyword (due to not using quoted "forward types").
The resulting (faulty) code looked like this (mind the missing forward references in RetrievalFilter):

  RetrievalFilter = TypedDict(
      "RetrievalFilter",
      {
          "equals": Optional[FilterAttribute],
          ...
          "andAll": Optional[RetrievalFilterList],
          ...
      },
      total=False,
  )
  RetrievalFilterList = List[RetrievalFilter]
  
  class FilterValue(TypedDict, total=False):
      pass
  
  class FilterAttribute(TypedDict, total=False):
      key: FilterKey
      value: FilterValue

Changes

  • Properly forwards the quote_types parameter in the code generation logic to ensure that the recursive type handling also works for exceptions and shapes with members named with Python keywords.

Testing

  • Tested the ASF code generation for bedrock using botocore==1.35.72:
    pip install botocore==1.35.72
    python3 -m localstack.aws.scaffold generate bedrock
    
  • Check that the resulting code is valid (uses forward references in RetrievalFilter):
    RetrievalFilter = TypedDict(
        "RetrievalFilter",
        {
            "equals": Optional["FilterAttribute"],
            "notEquals": Optional["FilterAttribute"],
            "greaterThan": Optional["FilterAttribute"],
            "greaterThanOrEquals": Optional["FilterAttribute"],
            "lessThan": Optional["FilterAttribute"],
            "lessThanOrEquals": Optional["FilterAttribute"],
            "in": Optional["FilterAttribute"],
            "notIn": Optional["FilterAttribute"],
            "startsWith": Optional["FilterAttribute"],
            "listContains": Optional["FilterAttribute"],
            "stringContains": Optional["FilterAttribute"],
            "andAll": Optional["RetrievalFilterList"],
            "orAll": Optional["RetrievalFilterList"],
        },
        total=False,
    )
    RetrievalFilterList = List[RetrievalFilter]
    
    
    class FilterValue(TypedDict, total=False):
        pass
    
    
    class FilterAttribute(TypedDict, total=False):
        key: FilterKey
        value: FilterValue

@alexrashed alexrashed added the semver: patch Non-breaking changes which can be included in patch releases label Dec 2, 2024
@alexrashed alexrashed added this to the 4.1 milestone Dec 2, 2024
@alexrashed alexrashed self-assigned this Dec 2, 2024
@alexrashed alexrashed requested a review from thrau as a code owner December 2, 2024 15:23
Copy link

github-actions bot commented Dec 2, 2024

S3 Image Test Results (AMD64 / ARM64)

  2 files  ±0    2 suites  ±0   4m 1s ⏱️ -15s
441 tests ±0  389 ✅ ±0   52 💤 ±0  0 ❌ ±0 
882 runs  ±0  778 ✅ ±0  104 💤 ±0  0 ❌ ±0 

Results for commit ea27811. ± Comparison against base commit 052b9bf.

♻️ This comment has been updated with latest results.

Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

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

LGTM! Nice find

@alexrashed alexrashed merged commit 353bd2e into master Dec 3, 2024
35 of 38 checks passed
@alexrashed alexrashed deleted the fix-asf-scaffold-recursive-typeddict branch December 3, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants