Fixed circular reference in HTTPConnection

[francis-clairicia]

❓ What kind of change does this PR introduce?

• 🐞 bug fix
• 🐣 feature
• 📋 docs update
• 📋 tests/coverage improvement
• 📋 refactoring
• 💥 other

📋 What is the related issue number (starting with #)

Resolves #746

❓ What is the current behavior? (You can also link to an open issue here)

The UNIX socket server's caching system for credentials involves a circular reference to HTTPConnection, even for an INET server.

❓ What is the new behavior (if this is a feature change)?

The cache system is still in place, but now uses private attributes.

📋 Other information:

📋 Contribution checklist:

(If you're a first-timer, check out
this guide on making great pull requests)

• I wrote descriptive pull request text above
• I think the code is well written
• I wrote good commit messages
• I have squashed related commits together after
  the changes have been approved
• Unit tests for the changes exist
• Integration tests for the changes exist (if applicable)
• I used the same coding conventions as the rest of the project
• The new code doesn't generate linter offenses
• Documentation reflects the changes
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences

---

This change is 

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.99%. Comparing base (ac9b6e5) to head (ff66b04).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #755      +/-   ##
==========================================
- Coverage   84.01%   83.99%   -0.03%     
==========================================
  Files          28       28              
  Lines        4235     4242       +7     
==========================================
+ Hits         3558     3563       +5     
- Misses        677      679       +2     

[webknjaz]

Looks like this implementation adds complexity by putting cache management into multiple places. Can you find a decorator-based solution? It might be okay to add a dependency if there's a good implementation in the wild.

[francis-clairicia]

I'm not sure to understand: where is the complexity?

A decorator-based implementation will result in the same issue than functools.lru_cache and there will be much more complexity in the decorator implementation in order not to create a circular defence than by simply have a private attribute.

I really don't see the issue here.

[webknjaz]

@francis-clairicia would using a @cached_property work? This would mean having a new API and we could keep the old methods uncached for compat.

[francis-clairicia]

@webknjaz This option seems good. The only drawback is that a @cached_property is writable, whereas @property is read-only by default.

If that's OK with you, then I can make the changes accordingly.

[webknjaz]

@francis-clairicia I didn't realize it was writable. But here's how we can make it read-only. Put this helper into a _functools.py, so that it lives in a dedicated space. Add a test/_functools_test.py to test the setter raising an error so that it all's got good test coverage.

>>> import functools as _ft
>>> class readonly_cached_property(_ft.cached_property):
...     def __set__(self, instance, value):
...         attr_name = self.attrname
...         raise AttributeError(f'Attribute {attr_name} is read-only and cannot be set.')
>>> class AttrsCls:
...     @cached_property
...     def cp(self):
...         return 42
...     @readonly_cached_property
...     def rocp(self):
...         return 84
>>> attro = AttrsCls()
>>> attro.rocp
84
>>> attro.cp
42
>>> attro.rocp = 3
Traceback (most recent call last):
  File "<python-input-42>", line 1, in <module>
    attro.rocp = 3
    ^^^^^^^^^^
  File "<python-input-37>", line 4, in __set__
    raise AttributeError(f'Attribute {attr_name} is read-only and cannot be set.')
AttributeError: Attribute rocp is read-only and cannot be set.

Additionally, make the change note a bit more specific. Referencing an abstract unnamed circular ref does not help a reader understand the context and the effect this has on them.

[webknjaz]

@francis-clairicia hey, should I expect you to get back to addressing the review request in this PR? It's okay if not — I'll take over eventually, just not sure about the timeline.