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.

Sign up for GitHub

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

Jump to bottom

[python] Cleanup ThreadPool with atexit rather than __del__ #5094

Merged
merged 6 commits into from
Jan 29, 2020
Merged

[python] Cleanup ThreadPool with atexit rather than __del__ #5094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b6140f3
[python] Cleanup ThreadPool with atexit rather than __del__
fabianvf Jan 23, 2020
a2e3676
Update petstore examples
fabianvf Jan 23, 2020
6880e3f
Test to ensure threadpool is cleaned up
fabianvf Jan 23, 2020
d8c5c55
Docs now encourage using the context manager
fabianvf Jan 23, 2020
70f8021
Regenerate docs
fabianvf Jan 23, 2020
1d179e0
Update samples
fabianvf Jan 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion modules/openapi-generator/src/main/resources/python/api_client.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{{>partial_header}}
from __future__ import absolute_import

import atexit
import datetime
from dateutil.parser import parse
import json
Expand Down Expand Up @@ -75,18 +76,27 @@ class ApiClient(object):
self.user_agent = '{{#httpUserAgent}}{{{.}}}{{/httpUserAgent}}{{^httpUserAgent}}OpenAPI-Generator/{{{packageVersion}}}/python{{/httpUserAgent}}'
self.client_side_validation = configuration.client_side_validation

def __del__(self):
def __enter__(self):
return self

def __exit__(self, exc_type, exc_value, traceback):
self.close()

def close(self):
if self._pool:
self._pool.close()
self._pool.join()
fabianvf marked this conversation as resolved.
Show resolved Hide resolved
self._pool = None
if hasattr(atexit, 'unregister'):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it looks like atexit.unregister doesn't exist in Python 2, so I added this guard

atexit.unregister(self.close)

@property
def pool(self):
"""Create thread pool on first request
avoids instantiating unused threadpool for blocking clients.
"""
if self._pool is None:
atexit.register(self.close)
self._pool = ThreadPool(self.pool_threads)
return self._pool

Expand Down
32 changes: 18 additions & 14 deletions modules/openapi-generator/src/main/resources/python/api_doc_example.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,26 @@ from pprint import pprint
{{#hasAuthMethods}}
Copy link
Contributor

@spacether spacether Jan 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Can you add a line above {{#hasAuthMethods}} instantiating configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe configuration is instantiated in the {{> python_doc_auth_partial}} snippet

# Defining host is optional and default to {{{basePath}}}
configuration.host = "{{{basePath}}}"
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}
# Enter a context with an instance of the API client
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't these blocks {{#hasAuthMethods}}/{{^hasAuthMethods}} identical except for setting
configuration.host?
Please move the identical block contents underneath the {{/hasAuthMethods}} on line 25

with {{{packageName}}}.ApiClient(configuration) as api_client:
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}(api_client)
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}
{{/hasAuthMethods}}
{{^hasAuthMethods}}
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}()
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}
# Enter a context with an instance of the API client
with {{{packageName}}}.ApiClient() as api_client:
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}(api_client)
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}
{{/hasAuthMethods}}

try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
```
24 changes: 13 additions & 11 deletions modules/openapi-generator/src/main/resources/python/common_README.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,20 @@ from pprint import pprint
{{> python_doc_auth_partial}}
# Defining host is optional and default to {{{basePath}}}
configuration.host = "{{{basePath}}}"
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}
# Enter a context with an instance of the API client
with {{{packageName}}}.ApiClient(configuration) as api_client:
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}(api_client)
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}

try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
```

## Documentation for API Endpoints
Expand Down
24 changes: 13 additions & 11 deletions ...es/openapi-generator/src/main/resources/python/python-experimental/README_common.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@ from pprint import pprint
{{> python_doc_auth_partial}}
# Defining host is optional and default to {{{basePath}}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to instantiate configuration here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe configuration is instantiated in the {{> python_doc_auth_partial}} snippet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually it looks like that snippet doesn't even exit in the experimental directory...

configuration.host = "{{{basePath}}}"
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}
# Enter a context with an instance of the API client
with {{{packageName}}}.ApiClient(configuration) as api_client:
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}(api_client)
{{#allParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} (default to {{{.}}}){{/defaultValue}}
{{/allParams}}

try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#allParams}}{{#required}}{{paramName}}{{/required}}{{^required}}{{paramName}}={{paramName}}{{/required}}{{#hasMore}}, {{/hasMore}}{{/allParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/-first}}{{/operation}}{{/operations}}{{/-first}}{{/apis}}{{/apiInfo}}
```

## Documentation for API Endpoints
Expand Down
12 changes: 11 additions & 1 deletion modules/openapi-generator/src/main/resources/python/python-experimental/api_client.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import absolute_import

import json
import atexit
import mimetypes
from multiprocessing.pool import ThreadPool
import os
Expand Down Expand Up @@ -74,18 +75,27 @@ class ApiClient(object):
# Set default User-Agent.
self.user_agent = '{{#httpUserAgent}}{{{.}}}{{/httpUserAgent}}{{^httpUserAgent}}OpenAPI-Generator/{{{packageVersion}}}/python{{/httpUserAgent}}'

def __del__(self):
def __enter__(self):
return self

def __exit__(self, exc_type, exc_value, traceback):
self.close()

def close(self):
if self._pool:
self._pool.close()
self._pool.join()
self._pool = None
if hasattr(atexit, 'unregister'):
atexit.unregister(self.close)

@property
def pool(self):
"""Create thread pool on first request
avoids instantiating unused threadpool for blocking clients.
"""
if self._pool is None:
atexit.register(self.close)
self._pool = ThreadPool(self.pool_threads)
return self._pool

Expand Down
86 changes: 45 additions & 41 deletions .../openapi-generator/src/main/resources/python/python-experimental/api_doc_example.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,51 +7,55 @@ from pprint import pprint
{{#hasAuthMethods}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a line above {{#hasAuthMethods}} instantiating configuration?

# Defining host is optional and default to {{{basePath}}}
configuration.host = "{{{basePath}}}"
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}({{{packageName}}}.ApiClient(configuration))
# Enter a context with an instance of the API client
with {{{packageName}}}.ApiClient(configuration) as api_client:
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}(api_client)
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't these blocks {{#hasAuthMethods}}/{{^hasAuthMethods}} identical except for setting
configuration.host?
Please move the identical block contents underneath the {{/hasAuthMethods}} on line 20

{{/hasAuthMethods}}
{{^hasAuthMethods}}
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}()
# Enter a context with an instance of the API client
with {{{packageName}}}.ApiClient(configuration) as api_client:
# Create an instance of the API class
api_instance = {{{packageName}}}.{{{classname}}}(api_client)
{{/hasAuthMethods}}
{{#requiredParams}}{{^defaultValue}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}
{{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} if omitted the server will use the default value of {{{defaultValue}}}{{/defaultValue}}
{{/optionalParams}}
{{#requiredParams}}
{{^hasMore}}
{{#requiredParams}}{{^defaultValue}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}
{{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}} = {{{example}}} # {{{dataType}}} | {{{description}}}{{^required}} (optional){{/required}}{{#defaultValue}} if omitted the server will use the default value of {{{defaultValue}}}{{/defaultValue}}
{{/optionalParams}}
{{#requiredParams}}
{{^hasMore}}

# example passing only required values which don't have defaults set
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}{{#hasMore}}, {{/hasMore}}{{/defaultValue}}{{/requiredParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/hasMore}}
{{/requiredParams}}
{{#optionalParams}}
{{^hasMore}}
# example passing only required values which don't have defaults set
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}{{#hasMore}}, {{/hasMore}}{{/defaultValue}}{{/requiredParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/hasMore}}
{{/requiredParams}}
{{#optionalParams}}
{{^hasMore}}

# example passing only required values which don't have defaults set
# and optional values
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}, {{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}}={{paramName}}{{#hasMore}}, {{/hasMore}}{{/optionalParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/hasMore}}
{{/optionalParams}}
{{^requiredParams}}
{{^optionalParams}}
# example passing only required values which don't have defaults set
# and optional values
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}({{#requiredParams}}{{^defaultValue}}{{paramName}}, {{/defaultValue}}{{/requiredParams}}{{#optionalParams}}{{paramName}}={{paramName}}{{#hasMore}}, {{/hasMore}}{{/optionalParams}}){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/hasMore}}
{{/optionalParams}}
{{^requiredParams}}
{{^optionalParams}}

# example, this endpoint has no required or optional parameters
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}(){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/optionalParams}}
{{/requiredParams}}
# example, this endpoint has no required or optional parameters
try:
{{#summary}} # {{{.}}}
{{/summary}} {{#returnType}}api_response = {{/returnType}}api_instance.{{{operationId}}}(){{#returnType}}
pprint(api_response){{/returnType}}
except {{{packageName}}}.ApiException as e:
print("Exception when calling {{classname}}->{{operationId}}: %s\n" % e)
{{/optionalParams}}
{{/requiredParams}}
```
24 changes: 13 additions & 11 deletions samples/client/petstore/python-asyncio/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,19 @@ from pprint import pprint

# Defining host is optional and default to http://petstore.swagger.io:80/v2
configuration.host = "http://petstore.swagger.io:80/v2"
# Create an instance of the API class
api_instance = petstore_api.AnotherFakeApi(petstore_api.ApiClient(configuration))
body = petstore_api.Client() # Client | client model

try:
# To test special tags
api_response = api_instance.call_123_test_special_tags(body)
pprint(api_response)
except ApiException as e:
print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)

# Enter a context with an instance of the API client
with petstore_api.ApiClient(configuration) as api_client:
# Create an instance of the API class
api_instance = petstore_api.AnotherFakeApi(api_client)
body = petstore_api.Client() # Client | client model

try:
# To test special tags
api_response = api_instance.call_123_test_special_tags(body)
pprint(api_response)
except ApiException as e:
print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)

```

## Documentation for API Endpoints
Expand Down
22 changes: 12 additions & 10 deletions samples/client/petstore/python-asyncio/docs/AnotherFakeApi.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,18 @@ import petstore_api
from petstore_api.rest import ApiException
from pprint import pprint

# Create an instance of the API class
api_instance = petstore_api.AnotherFakeApi()
body = petstore_api.Client() # Client | client model

try:
# To test special tags
api_response = api_instance.call_123_test_special_tags(body)
pprint(api_response)
except ApiException as e:
print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)
# Enter a context with an instance of the API client
with petstore_api.ApiClient() as api_client:
# Create an instance of the API class
api_instance = petstore_api.AnotherFakeApi(api_client)
body = petstore_api.Client() # Client | client model

try:
# To test special tags
api_response = api_instance.call_123_test_special_tags(body)
pprint(api_response)
except ApiException as e:
print("Exception when calling AnotherFakeApi->call_123_test_special_tags: %s\n" % e)
```

### Parameters
Expand Down
Loading