Add clarification to configuring app

[sha016]

What do these changes do?

Added clarification on configuring the app object with setings such as a db connection

Are there changes in behavior for the user?

No

Related issue number

Closes #4137

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • The format is <Name> <Surname>.
  • Please keep alphabetical order, the file is sorted by names.
• Add a new news fragment into the CHANGES folder
  • name it <issue_id>.<type> for example (588.bugfix)
  • if you don't have an issue_id change it to the pr id after creating the pr
  • ensure type is one of the following:
    • .feature: Signifying a new feature.
    • .bugfix: Signifying a bug fix.
    • .doc: Signifying a documentation improvement.
    • .removal: Signifying a deprecation or removal of public API.
    • .misc: A ticket has been closed, but it is not of interest to users.
  • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

[webknjaz]

Closes #4137

I don't believe this solves that issue. The issue asks for clarification and explanation about why, not how.

[sha016]

Would you agree that by saving the reference to the database connection in app['db'] it allows you to access it easily later in the request context via request.app['db']?

https://docs.aiohttp.org/en/stable/web_reference.html#application

Is that mainly why one should do this or are there other sections of code that look in app['db'] for a connection? I'm not intimately familiar with the codebase.

Thanks in advance,

[Arfey]

Now i don't think so that is the best solution for share connection with storage or something like that. Current pattern produce a lot of drilling arguments between functions

async def get_statistic(db, redis):
    data = await get_statistic(db)
    await inc_stats_view(redis)
    return data

async def index(req):
   await get_data(req.app['db'], req.app['redis])
   ...

it's not okey. in my opinion the best way is to create a context variable for db and redis connections and then use them in get_statistic and inc_stats_view .

I think so, problem #4137 in 2021 is not relevant, but I could be wrong

[Dreamsorcerer]

If you compare to other projects like flask, they often use global variables (flask.g etc.). This approach is less explicit and can be very error prone (my previous company's flask site was randomly serving Chinese translations to all visitors due to this, which could never have happened with the aiohttp approach).

So, I feel that the benefit is making sure everything is explicit, thus reducing the chance of bugs, by having the request/app object passed directly into the handler function and never using globals. The app object just gives us a convenient place to store these objects, but the main point is that everything is passed to the function explicitly.

It is also worth noting that we encourage the same approach with handling the app object itself. In many other frameworks the app object is created as a global variable in their examples. In all the aiohttp examples, the app object is always created as a local variable in a function (create_app() or similar). Again, this helps people to avoid problems which I've often seen on flask projects where the app object is getting imported into nearly every module (e.g. to use @app.get() decorators for handlers) and then ends up with circular imports and import ordering issues.

[asvetlov]

@Dreamsorcerer the perfect summary of my (never written maybe) thoughts when I designed this part of aiohttp.web API.

[sha016]

Maybe that was poor word choice calling the app object global. So the why to this issues is simply convenience?

[Dreamsorcerer]

Maybe that was poor word choice calling the app object global. So the why to this issues is simply convenience?

No, reread my comment, I was not responding to you calling it a global, but explaining the why. We should be explaining why we use the app object in this way, as opposed to the way other popular frameworks do it (i.e. avoiding globals etc.). If convenience was the only factor, we would just be lazy and use globals like flask.

The app object is a convenient location for storing these things given the design parameters of aiohttp. But, it's those design parameters which are the reason we end up using it. So, any documentation for this should probably mention globals and explicitness, and how this reduces the chances of bugs in code.

[codecov]

Codecov Report

Merging #6163 (ed0a2fd) into master (00d5286) will decrease coverage by 0.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6163      +/-   ##
==========================================
- Coverage   93.31%   93.13%   -0.19%     
==========================================
  Files         102      103       +1     
  Lines       30212    30368     +156     
  Branches     2708     2730      +22     
==========================================
+ Hits        28192    28282      +90     
- Misses       1843     1896      +53     
- Partials      177      190      +13     

Flag	Coverage Δ
unit	93.06% <ø> (-0.18%)	⬇️

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

Impacted Files	Coverage Δ
tests/test_loop.py	95.12% <0.00%> (-4.88%)	⬇️
tests/conftest.py	64.95% <0.00%> (-3.42%)	⬇️
aiohttp/http_websocket.py	95.96% <0.00%> (-2.96%)	⬇️
aiohttp/web.py	96.63% <0.00%> (-2.53%)	⬇️
tests/test_client_session.py	97.40% <0.00%> (-2.09%)	⬇️
tests/test_multipart.py	98.46% <0.00%> (-1.54%)	⬇️
aiohttp/worker.py	26.98% <0.00%> (-1.35%)	⬇️
aiohttp/client_proto.py	95.33% <0.00%> (-1.34%)	⬇️
aiohttp/helpers.py	96.31% <0.00%> (-0.61%)	⬇️
aiohttp/http_parser.py	92.03% <0.00%> (-0.57%)	⬇️
... and 28 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00d5286...ed0a2fd. Read the comment docs.

[sha016]

@Dreamsorcerer Thanks for explaining your thoughts @asvetlov I've amended the commit and it should cover what we've discussed here.

[webknjaz]

@Arfey WDYT?

[Dreamsorcerer]

Looks OK to me.

[patchback]

Backport to 3.8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.8/a0a9b3db486df1ea2aed21dc6c9c1d72452d31fc/pr-6163

Backported as #6341

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[patchback]

Backport to 3.9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.9/a0a9b3db486df1ea2aed21dc6c9c1d72452d31fc/pr-6163

Backported as #6342

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.