Skip to content

Fix hyphens in path parameters #986

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 7 commits into from
Mar 6, 2024

Conversation

harabat
Copy link
Contributor

@harabat harabat commented Mar 3, 2024

Fixes #976 and #578, and replaces #978.

@dbanty please choose your preferred approach between this and PR #987.

The original issue is that openapi-python-client throws incorrect path templating warnings when the path has a parameter with a hyphen and consequently fails to generate the endpoints.


The first commit ensures that hyphens are recognised as allowed delimiters in parameter path names. This allows the endpoints to be generated.

However, this generates lines like these:

def _get_kwargs(
    user_id: int,
) -> Dict[str, Any]:
    _kwargs: Dict[str, Any] = {
        "method": "post",
        "url": "/activitypub/user-id/{user-id}/inbox".format(user-id=user_id,),
    }
    return _kwargs

Since Python variable names cannot contain hyphens, the user-id parameter name here will trigger errors (starting with ruff).


The second commit replaces parameter names with their python_name in __init__.py and passes the modified path to templates/endpoint_module.py.jinja.

This fixes the issue and allows endpoints to be generated correctly.


#987 is a different option for the second commit which instead creates a custom Jinja filter in utils.py and so that the parameter names in endpoint.path can be converted to their python names directly in templates/endpoint_module.py.jinja.

Both approaches are equivalent and have been tested with different parameter names (snake case, camel case, kebab case, mixed).

@harabat harabat marked this pull request as ready for review March 3, 2024 12:51
@harabat harabat force-pushed the endpoint_path_python_names branch from ebb9662 to c58b601 Compare March 3, 2024 17:01
@harabat harabat force-pushed the endpoint_path_python_names branch from c58b601 to bd28cf7 Compare March 3, 2024 17:04
@harabat harabat changed the title add path with python names as variable to template fix: add path with python names as variable to template Mar 3, 2024
@dbanty dbanty changed the title fix: add path with python names as variable to template Fix hyphens in path parameters Mar 6, 2024
Copy link
Collaborator

@dbanty dbanty left a comment

Choose a reason for hiding this comment

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

Wow, not just a fix for the bug, but 2 choices! Thanks a bunch ☺️

@dbanty dbanty enabled auto-merge March 6, 2024 02:13
@dbanty dbanty added this pull request to the merge queue Mar 6, 2024
Merged via the queue into openapi-generators:main with commit 88ffbd6 Mar 6, 2024
20 checks passed
dbanty added a commit that referenced this pull request Mar 6, 2024
This PR was created by Knope. Merging it will create a new release

### Breaking Changes

#### Update PDM metadata syntax

Metadata generated for PDM will now use the new `distribution = true`
syntax instead of `package-type = "library"`.
New packages generated with `--meta pdm` will require PDM `2.12.0` or
later to build.

### Features

#### Add response content to `UnexpectedStatus` exception

The error message for `UnexpectedStatus` exceptions will now include the
UTF-8 decoded (ignoring errors) body of the response.

PR #989 implements #840. Thanks @harabat!

### Fixes

#### Allow hyphens in path parameters

Before now, path parameters which were invalid Python identifiers were
not allowed, and would fail generation with an
"Incorrect path templating" error. In particular, this meant that path
parameters with hyphens were not allowed.
This has now been fixed!

PR #986 fixed issue #976. Thanks @harabat!

> [!WARNING]
> This change may break custom templates, see [this
diff](https://github.com/openapi-generators/openapi-python-client/pull/986/files#diff-0de8437b26075d8fe8454cf47d8d95d4835c7f827fa87328e03f690412be803e)
> if you have trouble upgrading.

Co-authored-by: GitHub <[email protected]>
@harabat
Copy link
Contributor Author

harabat commented Mar 6, 2024

Thanks @dbanty!

@harabat harabat deleted the endpoint_path_python_names branch March 6, 2024 10:34
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.

Endpoints with hyphens in parameter names trigger Incorrect path templating warning, are not generated
2 participants