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

feat(python): handle multiple file parameters #19329

Conversation

roryschadler
Copy link
Contributor

Both library=urllib3 and library=asyncio Python clients fail to handle multiple files, despite the type annotations implying they should. To see this, generate both clients using latest master and modules/openapi-generator/src/test/resources/3_0/form-multipart-binary-array.yaml. The /multipart-array calls fail in both clients with ValueError: Unsupported file value

import asyncio
from example_urllib_client.api_client import ApiClient as UrllibApiClient
from example_urllib_client.configuration import Configuration as UrllibConfiguration
from example_urllib_client.api.multipart_api import MultipartApi as UrllibMultipartApi
from example_asyncio_client.api_client import ApiClient as AsyncioApiClient
from example_asyncio_client.configuration import Configuration as AsyncioConfiguration
from example_asyncio_client.api.multipart_api import MultipartApi as AsyncioMultipartApi


asyncio_configuration = AsyncioConfiguration(host="http://localhost:8008")

urllib_configuration = UrllibConfiguration(host="http://localhost:8008")


async def asyncio_main():
    async with AsyncioApiClient(asyncio_configuration) as asyncio_api_client:
        asyncio_api_instance = AsyncioMultipartApi(asyncio_api_client)
        with open("hello-world.txt", "rb") as f:
            file1 = f.read()

        with open("lorem-ipsum.txt", "rb") as f:
            file2 = f.read()

        try:
            await asyncio_api_instance.multipart_array(
                files=[file1, file2],
            )
        except ValueError as e:
            print(f"asyncio.multipart_array: {type(e).__name__}: {e}")


def urllib_main():
    with UrllibApiClient(urllib_configuration) as urllib_api_client:
        urllib_api_instance = UrllibMultipartApi(urllib_api_client)
        with open("hello-world.txt", "rb") as f:
            file1 = f.read()

        with open("lorem-ipsum.txt", "rb") as f:
            file2 = f.read()

        try:
            urllib_api_instance.multipart_array(
                files=[file1, file2],
            )
        except ValueError as e:
            print(f"urllib.multipart_array: {type(e).__name__}: {e}")


if __name__ == "__main__":
    asyncio.run(asyncio_main())
    urllib_main()

This change provides two solutions: one, it handles lists of filenames or lists of bytestrings, as the API's type annotation suggested it should. Two, it adds the ability to pass tuples of filename/bytestring pairs.

There isn't currently a test which obviously checks this - if I'm missing something, please let me know!

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh ./bin/configs/*.yaml
    ./bin/utils/export_docs_generators.sh
    
    (For Windows users, please run the script in Git BASH)
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
  • File the PR against the correct branch: master (upcoming 7.6.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Previously an array of files was not handled correctly, despite the type
annotation implying it was.
This allows for users to pass filenames with their data in file params,
which is useful for multipart formdata requests. Without this, the list
of files added in the previous commit would have the same filename for
all files (the parameter name).
@wing328
Copy link
Member

wing328 commented Aug 12, 2024

can you please add a fake endpoint to modules/openapi-generator/src/test/resources/3_0/python/petstore-with-fake-endpoints-models-for-testing.yaml similar to https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/python/petstore-with-fake-endpoints-models-for-testing.yaml#L581 ?

@roryschadler
Copy link
Contributor Author

@wing328 Sorry for the slow response, I've been out this week. I'm happy to add another endpoint, but it looks like this one already includes an array of files (schema). Is that sufficient?

@roryschadler
Copy link
Contributor Author

@wing328 bumping this one as well - WDYT about that existing fake endpoint?

@wing328
Copy link
Member

wing328 commented Aug 20, 2024

@roryschadler have you tested the fix locally with your own spec to confirm the issue has been fixed?

@roryschadler
Copy link
Contributor Author

I have, yes. All works well!

@kannwism
Copy link

I'm facing the same issue and can confirm that a local build of this PR fixes the issue for our spec as well :)

@wing328
Copy link
Member

wing328 commented Aug 21, 2024

thanks. let's give it a try

@wing328 wing328 merged commit cc98333 into OpenAPITools:master Aug 21, 2024
33 checks passed
@wing328 wing328 added this to the 7.9.0 milestone Aug 21, 2024
@roryschadler roryschadler deleted the roryschadler/feat-multiple-file-parameters-python branch August 21, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants