Added gzip as default request header

[rsaggino]

Fixes #273 by adding gzip headers to the api client.

Tested on a small dev cluster, nipyapi 0.16.2 and NiFi 1.11.
It seems like there were additional improvements on NiFi/nipyapi that reduced the impact of this patch, this is still necessary on larger clusters and while making complex automations.

Before:

In []: %%time
   ...: import nipyapi
   ...: a=nipyapi.canvas.list_all_processors()
   ...:
   ...:
CPU times: user 22.3 s, sys: 3.46 s, total: 25.8 s
Wall time: 1min 27s

In []: %%time
   ...: import nipyapi
   ...: a=nipyapi.canvas.list_all_controllers()
   ...:
   ...:
CPU times: user 12.1 s, sys: 2.24 s, total: 14.3 s
Wall time: 35.7 s

After:

In []: %%time
   ...: import nipyapi
   ...: a=nipyapi.canvas.list_all_processors()
   ...:
   ...:
CPU times: user 19.8 s, sys: 1.11 s, total: 20.9 s
Wall time: 1min 1s

In []: %%time
   ...: import nipyapi
   ...: a=nipyapi.canvas.list_all_controllers()
   ...:
   ...:
CPU times: user 11.4 s, sys: 481 ms, total: 11.9 s
Wall time: 19.6 s

[coveralls]

Coverage increased (+0.3%) to 69.11% when pulling 64e2a9d on rsaggino:gzip_nifi_api into 292aa1e on Chaffelson:main.

[ottobackwards]

swagger-api/swagger-codegen#10085

So, this is a problem with the code gen.
I'm not sure we would want to change this file itself, since it is generated.

Normally for issues such as this, I would apply a patch post generation. Thoughts @Chaffelson ?

[ottobackwards]

Note, that given url lib support for gzip, this is the right fix for the generated code
It is just how it is managed.

[rsaggino]

Normally for issues such as this, I would apply a patch post generation. Thoughts @Chaffelson ?

That would be the best option (aside from fixing this upstream).
Is the post-gen scripted somewhere or completely manual?

[ottobackwards]

https://github.com/Chaffelson/nipyapi/tree/main/resources/client_gen

[Chaffelson]

The client is primarily generated using mustache templates, so if this was added to the template then all future clients would pick it up.

nipyapi/resources/client_gen/swagger_templates/api_client.mustache

Line 58 in 292aa1e

self.default_headers = {}

[rsaggino]

The client is primarily generated using mustache templates, so if this was added to the template then all future clients would pick it up.

nipyapi/resources/client_gen/swagger_templates/api_client.mustache

Line 58 in 292aa1e

self.default_headers = {}

This is an easy fix but could get lost in future updates of the templates. Any preference?

[Chaffelson]

They are my templates, not upstream, so it won't be lost

…

On Fri, 11 Jun 2021, 15:57 rsaggino, ***@***.***> wrote: The client is primarily generated using mustache templates, so if this was added to the template then all future clients would pick it up. https://github.com/Chaffelson/nipyapi/blob/292aa1e8bc1c4d080d7b6acdf0cedfb4e77a1f03/resources/client_gen/swagger_templates/api_client.mustache#L58 This is an easy fix but could get lost in future updates of the templates. Any preference? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#277 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACZAZOBJY3I7VLFKYZZUTFLTSIP4RANCNFSM46A4TE7A> .

[ottobackwards]

@Chaffelson maybe you can list out explicitly what we would want to do?

[ottobackwards]

ie:

• update template in pr
• generate using scripts
• cut a release
• ???
• profit?

[rsaggino]

Applied the change also to the mustache template.
Given the idea to use Bravado in the future, we could leave the generate script aside.

[rsaggino]

@Chaffelson can we proceed with this?

[ottobackwards]

I'm +1 on this, pending whatever testing @Chaffelson does for these kinds of changes

[ottobackwards]

fwiw
:)

[Chaffelson]

Yeah this is the way to do it, I was going to cut a release this week but am dealing with unexpected covid isolation so it'll have to be next week sorry.

[Chaffelson]

Managed to get some testing on this done today and it's looking good, I'll be rolling up the new client for 1.13 in the next week and including this in it