Skip to content

chore: revert custom changes to autogenerated code #181

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

Merged
merged 3 commits into from
Sep 21, 2024

Conversation

awalker4
Copy link
Collaborator

@awalker4 awalker4 commented Sep 20, 2024

The issue

The v2 python client has some minor breaking changes for our users, both related to keyword parameters:

  • The PartitionRequest class is now a pydantic model which does not support positional arguments in the constructor. Therefore we always have to name the partition_parameters field.
    req = operations.PartitionRequest(
        shared.PartitionParameters(
            ....
            ....
        )
    )

becomes

    req = operations.PartitionRequest(
        partition_parameters=shared.PartitionParameters(
            ....
            ....
        )
    )

  • Likewise, the function signature for general.partition no longer allows for positional args.

Instead of

    response = client.general.partition(req)

we have to do

    response = client.general.partition(request=req)

The first approach

These are minimal changes, but they will disrupt a number of users who have copied our sample snippets in the past. My workaround for this was to manually patch the autogenerated code to bring back positional params. However, this will be messy in the long term, as we now have to apply our changes after every regenerate. Instead, let's embrace the version bump to 0.16.0 and just document the breaking change.

Update: The python snippet in the serverless dashboard uses keyword args, so users who copied this out are unaffected.

Changes

  • Revert my manual edits to:
    • Allow request as a positional arg in general.partition (general.py)
    • Add a constructor to PartitionRequest so we don't have to use a keyword arg
  • Remove the patch file containing these changes
  • Update some makefile commands to reuse in other repos

Copy link
Contributor

@Coniferish Coniferish left a comment

Choose a reason for hiding this comment

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

LGTM

@awalker4 awalker4 enabled auto-merge (squash) September 21, 2024 01:12
@awalker4 awalker4 merged commit 643ed88 into main Sep 21, 2024
7 checks passed
@awalker4 awalker4 deleted the fix/fix-internal-tests branch September 21, 2024 01:39
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.

2 participants