Skip to content

python: remove non-async code path from the aiohttp generator#16601

Merged
wing328 merged 3 commits intoOpenAPITools:masterfrom
multani:fix-15824
Sep 20, 2023
Merged

python: remove non-async code path from the aiohttp generator#16601
wing328 merged 3 commits intoOpenAPITools:masterfrom
multani:fix-15824

Conversation

@multani
Copy link
Copy Markdown
Contributor

@multani multani commented Sep 16, 2023

This removes all the non-async code from the aiohttp generator:

  • all the methods that should be asynchronous are marked as async
  • the async_req parameter is gone; calls are directly awaitables now
  • the async calls into a thread pool are gone and the thread pool is gone too (now useless)

Closes: #15824
Closes: #5539
Related: #763
Related: #3696

The code is extracted from #15878, readjusted to remove the typing changes and fixed for the aiohttp client tests to pass.

@krjakbrjak @wing328

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/utils/export_docs_generators.sh
    
    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*.
    For Windows users, please run the script in Git BASH.
  • In case you are adding a new generator, run the following additional script :
    ./bin/utils/ensure-up-to-date
    
    Commit all changed files.
  • File the PR against the correct branch: master (7.0.1 - patch release), 7.1.x (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.

This removes all the non-async code from the aiohttp generator:

* all the methods that should be asynchronous are marked as `async`
* the `async_req` parameter is gone; calls are directly awaitables now
* the async calls into a thread pool are gone and the thread pool is
  gone too (now useless)

Closes: OpenAPITools#15824
Closes: OpenAPITools#5539
Related: OpenAPITools#763
Related: OpenAPITools#3696
@multani
Copy link
Copy Markdown
Contributor Author

multani commented Sep 16, 2023

I noticed GitHub Actions doesn't run tests for the client in https://github.com/OpenAPITools/openapi-generator/tree/master/samples/openapi3/client/petstore/python-aiohttp: I can also take a look to add a new workflow to test this specific client, just let me know.

@multani
Copy link
Copy Markdown
Contributor Author

multani commented Sep 16, 2023

I noticed GitHub Actions doesn't run tests for the client in master/samples/openapi3/client/petstore/python-aiohttp: I can also take a look to add a new workflow to test this specific client, just let me know.

Or another sample client (like an aiohttp-based echo API client for instance)

@wing328 wing328 added Client: Python Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. labels Sep 20, 2023
@wing328 wing328 added this to the 7.0.1 milestone Sep 20, 2023
@wing328
Copy link
Copy Markdown
Member

wing328 commented Sep 20, 2023

FYI @taxpon (2017/07) @frol (2017/07) @mbohlool (2017/07) @cbornet (2017/09) @kenjones-cisco (2017/11) @tomplus (2018/10) @arun-nalla (2019/11) @krjakbrjak (2023/02)

@wing328
Copy link
Copy Markdown
Member

wing328 commented Sep 20, 2023

looks good to me.

the change seems to be transparent to end uesrs consuming the python sdk as all tests still pass without any need to update the test.

@wing328 wing328 merged commit a2f6b8e into OpenAPITools:master Sep 20, 2023
@multani multani deleted the fix-15824 branch September 21, 2023 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client: Python Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. Issue: Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Why is the python asyncio generator is generating a client with a threadpool [BUG][Python] async_req=True doesn't work with asyncio

2 participants