Fix: close client session to avoid resource leak

[guyskk]

Fix requests session not closed which will cause resource leak.

[guyskk]

Hi @matin, any feedbacks on the pull request?

[matin]

@guyskk thanks for the contribution! This approach would definitely work, but what about the following approach instead?

from datetime import date

from cep import Transferencia

tr_details = dict(
    fecha=date(2019, 4, 12),
    clave_rastreo='CUENCA1555093850',
    emisor='90646',  # STP
    receptor='40012',  # BBVA
    cuenta='012180004643051249',
    monto=8.17,
)
with Transferencia.validar(**tr_details) as tr:
    pdf = tr.descargar()

Using contextlib for the above would make it more consistent with how sessions, clients, etc that need to be closed after an operation (successful or failed) are typically mitigated. It also removes the need to manually call Client.close().

What do you think?

[guyskk]

@matin The problem is Transferencia.validar may return None, which cause not able to use context manager.

If we can move validar and descargar to another class (or client class), then use context manager is straightforward. eg:

with TransferenciaClient() as client:
    tr = client.validar(**tr_details)
    pdf = tr.descargar()

But it may be a break change, or keep the old api, only deprecate them.

[guyskk]

Hi @matin, I implemented transferencia client use context manager, and reserved all existing api, only deprecated them in docs. Could you help review again?

[matin]

Hi @matin, I implemented transferencia client use context manager, and reserved all existing api, only deprecated them in docs. Could you help review again?

Looks good to me.

@rogelioLpz can you review as well?

[matin]

@guyskk can you bump the version as well?

[guyskk]

@matin bumped version from 0.1.7 to 0.2.0, increased minor version because the PR add new APIs.

[matin]

Leaving to @rogelioLpz for final review

[rogelioLpz]

Hi @guyskk and thanks for the contribution!
Not is necessary maintain the both class Transferencia and TransferenciaClient

Please apply your changes in Transferencia and delete TransferenciaClient

[guyskk]

Hi @rogelioLpz , the Transferencia.validar may return None, which cause not able to use context manager, so we have to add another class. #70 (comment)

[guyskk]

Hi @rogelioLpz , any suggestions or feedbacks on the PR?