Reconsider the Connection and Pool classes APIs -> preparing v0.3.0 release

[stankudrow]

Inspired by the issue #127 .

Major compability-breaking features:

• Connection
  • remove the deprecated connected attribute in favour of opened property
  • turn the connect function into asyncontextmanager
  • make the close async method more consistent
• Pool:
  • check pool connection freshness when acquiring and releasing connections from/to the pool
  • turn the create_pool function into asyncontextmanager
  • the pool module is linted: mypy got happy when removing the asyncio.AbstractServer inheritance
• common:
  • README updated
  • CHANGELOG updated -> v0.3.0
  • some connection/pool unit tests added

It is good to have this PR merged after accepting the PR #128 .
Together with this PR and before the release v0.3.0, the PR #129 has to be resolved.

UPD: relates to the issue #126 .

[stankudrow]

@nils-borrmann-tacto, hello and I invite you to review this PR.

@DaniilAnichin , @DFilyushin, @KuzenkovAG, if you have time, your reviews are very welcome and appreciated. Also I take the liberty to draw your attention to the PR #129 .

[nils-borrmann-tacto]

Hey! What's your plan with this PR and my PR? Are you planning to merge both of them? As far as I can see this change doesn't address any of the issues from my PR. The refresh() method is basically a copy of the old force_connect(), but doesn't change its behaviour.

If we plan to merge both of them, it would probably be better to move the at_eof() check into the refresh(). And thinking about it, we probably want a better way of dealing with a None packet. Maybe make that an exception?

[stankudrow]

Hey! What's your plan with this PR and my PR? Are you planning to merge both of them? As far as I can see this change doesn't address any of the issues from my PR. The refresh() method is basically a copy of the old force_connect(), but doesn't change its behaviour.

If we plan to merge both of them, it would probably be better to move the at_eof() check into the refresh(). And thinking about it, we probably want a better way of dealing with a None packet. Maybe make that an exception?

I am planning to work together on your PR #129 , that is why I asked you to send me invitation to your repository.

I thought that a refresh method is nice to have in the Connection class public API (the PR #130 ). Then, I guess, using refresh method in a Pool instance is a way to go when giving a free connection from the Pool: a dangling one should not be returned (a next PR). And all of this thanks to you.

So now the PR #129 and then theses two above.

[nils-borrmann-tacto]

Honestly, I'm not sure about having a public refresh() api. As a user of the connection pool, I don't want to have to worry about internals or aborted connections. The library should just handle this for me, and re-establish the connection.

[stankudrow]

Honestly, I'm not sure about having a public refresh() api. As a user of the connection pool, I don't want to have to worry about internals or aborted connections. The library should just handle this for me, and re-establish the connection.

I guess you are right, so it may be a private one.
I'll inject refreshing logic into the Pool class and see how useful this pre-check can be.

Thank you.

[stankudrow]

@nils-borrmann-tacto , I implemented Connection refreshment logic for validating a connection before it leaves a pool and when it comes back. Thank you for your opinion and feel free to share your thoughts about the feature. When free, I'll find time for your PR #129 .

@DaniilAnichin , I requested yet another review on new features, please have a look at them any time.

[nils-borrmann-tacto]

Looks good now imo 👍

[stankudrow]

@DaniilAnichin , hello. I need your rereview, some changes got after and I don't want them to pass silently.

[stankudrow]

@nils-borrmann-tacto , @DaniilAnichin , hello again. The PR is rebased, could you possibly look at it once more? Just a last check before asking the owner to merge it.

@DFilyushin, @KuzenkovAG , could you take part as well? Your reviews are appreciated.

Thank you all in advance.

[stankudrow]

@nils-borrmann-tacto , @pohmelie , @DaniilAnichin , @tvorogme, @KuzenkovAG, @DFilyushin, @RuslanUsmanov, @long2ice , hello, guys.

I need your review again. Feel free to coathor in any way by, e.g., suggesting commits and missing unit tests especially.

Thank you for your attention and efforts.

[stankudrow]

@nils-borrmann-tacto , @DaniilAnichin , @pohmelie , @tvorogme, @long2ice , hello again.

Ready for review again and perhaps finally. Feel free to share your ideas, suggest commits on the code and tests and so forth.

If no review in two weeks, I'll ask the owner of the repository to merge this feature and this should open the way to the v0.3.0.

[DaniilAnichin]

In general still looks good, but since the changes are being introduced into opened & closed checks, I think it may be worth to just use status (as mentioned in respective comment)

[stankudrow]

@DaniilAnichin , @pohmelie , @nils-borrmann-tacto , hello again...and I request your help again. Two approvals will do and then I ask the owner to merge this PR.

Thank you for your time, patience and openness, I appreciate your help and contribution.

[stankudrow]

@long2ice , hello.

Two approvals got, the PR is ready for final preps.

[long2ice]

ThanksQ