Make AppKey compatible with string keys

[cdce8p]

What do these changes do?

Allow the simultaneous use of AppKey and string keys. During my early testing for Home Assistant I found it quite challenging to start using AppKey. At least in this particular case, a variable was stored once but read all throughout the code base. Just missing one update, would cause a test failure. By making both compatible, users will be able to update them gradually.

The only downside being that the names of AppKey will no longer be able to contain the module name and loose some of their uniqueness. However, I don't believe that is an issue with the current string keys so it should be fine. It might even help avoid some additional confusions if AppKey instances have the same name + same variable name but are in different modules -> so not compatible.

Overall, I found that without this change, I wasn't able to introduce AppKey during testing in a larger code base.

/CC @Dreamsorcerer

Are there changes in behavior for the user?

The feature hasn't been shipped yet.

[codecov]

Codecov Report

Merging #7555 (f3331e0) into master (cf97e5b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #7555   +/-   ##
=======================================
  Coverage   97.35%   97.36%           
=======================================
  Files         106      106           
  Lines       31481    31488    +7     
  Branches     3575     3574    -1     
=======================================
+ Hits        30648    30657    +9     
+ Misses        630      629    -1     
+ Partials      203      202    -1     

Flag	Coverage Δ
CI-GHA	97.31% <100.00%> (+<0.01%)	⬆️
OS-Linux	96.98% <100.00%> (+<0.01%)	⬆️
OS-Windows	95.45% <100.00%> (+<0.01%)	⬆️
OS-macOS	96.65% <100.00%> (+<0.01%)	⬆️
Py-3.10.11	95.38% <100.00%> (+<0.01%)	⬆️
Py-3.10.12	96.87% <100.00%> (+<0.01%)	⬆️
Py-3.11.4	96.56% <100.00%> (+<0.01%)	⬆️
Py-3.8.10	95.35% <100.00%> (+<0.01%)	⬆️
Py-3.8.17	96.80% <100.00%> (+<0.01%)	⬆️
Py-3.9.13	95.34% <100.00%> (+<0.01%)	⬆️
Py-3.9.17	96.83% <100.00%> (+<0.01%)	⬆️
Py-pypy7.3.11	96.37% <100.00%> (+<0.01%)	⬆️
VM-macos	96.65% <100.00%> (+<0.01%)	⬆️
VM-ubuntu	96.98% <100.00%> (+<0.01%)	⬆️
VM-windows	95.45% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
aiohttp/helpers.py	95.95% <100.00%> (+0.33%)	⬆️
tests/test_web_app.py	99.49% <100.00%> (+0.01%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[Dreamsorcerer]

Thanks for testing this out.

During my early testing for Home Assistant I found it quite challenging to start using AppKey. At least in this particular case, a variable was stored once but read all throughout the code base. Just missing one update, would cause a test failure.

Could you show an example of what the problem is? I'd have thought that it would just be a find/replace job...

_The only downside being that the names of AppKey will no longer be able to contain the module name and loose some of their uniqueness. However, I don't believe that is an issue with the current string keys so it should be fine.

The problem this solves is that libraries with string keys have needed to use awkward naming conventions to reduce the likelihood (but not guarantee) that the keys won't clash with a user's own keys or those of another library. With AppKey, this is no longer a problem, but displaying the module name is needed to tell which key we're talking about. e.g. Your application might have a "db" key, aiohttp-admin might have a "db" key and aiohttp-session might have a "db" key. That's 3 different keys that could share the same name (currently, they use awkward names like "__aiohttp_session_db", but a library from another developer not aware of the issue could easily use a simpler string without realising that it's likely to clash with a portion of their user's own keys).

It might even help avoid some additional confusions if AppKey instances have the same name + same variable name but are in different modules -> so not compatible._

As above, that's rather the point, they are different keys. It would be more confusing to find your key now refers to aiohttp-admin's database connection and not the one you created in your application.

[cdce8p]

During my early testing for Home Assistant I found it quite challenging to start using AppKey. At least in this particular case, a variable was stored once but read all throughout the code base. Just missing one update, would cause a test failure.

Could you show an example of what the problem is? I'd have thought that it would just be a find/replace job...

Find and replace technically, however we don't control all parts. There are a lot of external component which we can't update directly / need to be updated by their developers after a release. Just starting to use AppKey would thus be a breaking change.

# -- we control this --
class HomeAssistant: ...
KEY_HASS = AppKey[HomeAssistant]("hass")
hass = HomeAssistant()

 app = web.Application()
-app["hass"] = hass
+app[KEY_HASS] = hass

# -- external components --
# this would break without an update
hass = app["hass“]

That's where the idea to make str compatible with AppKey came from.

The problem this solves is that libraries with string keys have needed to use awkward naming conventions to reduce the likelihood (but not guarantee) that the keys won't clash with a user's own keys or those of another library. [...]

As above, that's rather the point, they are different keys. It would be more confusing to find your key now refers to aiohttp-admin's database connection and not the one you created in your application.

Is this such a prevent issue? For db sure, but other keys as well? I would consider it good practice to choose a unique name and if that means to prefix it with the application name, so be it. Is it that nobody is doing it atm, so AppKey should do the name mangling automatically?

--
🤔 Not an easy solution here. I'd have liked to be able to do a one-to-one replacement, but maybe that isn't possible given the circumstances. To avoid the breaking change for Home Assistant then, I should probably assign both a str key and an AppKey (and deprecated the str key as per usual).

app[KEY_HASS] = app["hass"] = hass

[Dreamsorcerer]

so AppKey should do the name mangling automatically?

It's not mangling the name, the object itself is the key. It's just the repr() that adds the module name in to help debugging.

-- 🤔 Not an easy solution here. I'd have liked to be able to do a one-to-one replacement, but maybe that isn't possible given the circumstances. To avoid the breaking change for Home Assistant then, I should probably assign both a str key and an AppKey (and deprecated the str key as per usual).

app[KEY_HASS] = app["hass"] = hass

Yeah, I feel like duplicating it on the string key makes the most sense for transitioning.

[cdce8p]

so AppKey should do the name mangling automatically?

It's not mangling the name, the object itself is the key. It's just the repr() that adds the module name in to help debugging.

The repr name is mangled with the the module name but you're right the key comparison is based on object id.

-- 🤔 Not an easy solution here. I'd have liked to be able to do a one-to-one replacement, but maybe that isn't possible given the circumstances. To avoid the breaking change for Home Assistant then, I should probably assign both a str key and an AppKey (and deprecated the str key as per usual).

app[KEY_HASS] = app["hass"] = hass

Yeah, I feel like duplicating it on the string key makes the most sense for transitioning.

Not as beautiful as I would've liked but I the reasoning for keeping it the way it is makes sense. I'll go ahead and close this then.

[Dreamsorcerer]

The repr name is mangled with the the module name but you're right the key comparison is based on object id.

Sure, but really, this is just adding more information than we had before with string keys. If you print() the key object (e.g. when iterating through all keys in the Application), it should give you a very good idea of where that key was created and help you to find it a lot easier.