Test if CookieJar pickle format breaks

[Dreamsorcerer]

Fixes #7216 (well, stops it from happening accidentally again).

[Traktormaster]

Do I understand correctly that the save/load functionality will not be patched to be backward compatible with itself?

That would seem antithetical to their purpose of persisting the value and being able to use it later. In fact, breaking changes would require the using application to migrate the data at which point it could be easier to write a custom save/load for itself.

I feel like this is something the library should handle and not each application itself that uses it.

[codecov]

Codecov Report

Merging #7221 (4ee9572) into master (82ee406) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #7221      +/-   ##
==========================================
+ Coverage   97.25%   97.33%   +0.08%     
==========================================
  Files         106      106              
  Lines       31300    31309       +9     
  Branches     3216     3891     +675     
==========================================
+ Hits        30442    30476      +34     
+ Misses        656      634      -22     
+ Partials      202      199       -3     

Flag	Coverage Δ
CI-GHA	97.24% <100.00%> (+0.08%)	⬆️
OS-Linux	96.89% <100.00%> (+0.10%)	⬆️
OS-Windows	95.32% <100.00%> (+<0.01%)	⬆️
OS-macOS	96.48% <100.00%> (?)
Py-3.10.10	97.00% <100.00%> (+0.19%)	⬆️
Py-3.11.0	96.42% <100.00%> (?)
Py-3.7.15	96.71% <44.44%> (+0.10%)	⬆️
Py-3.7.9	95.19% <44.44%> (-0.02%)	⬇️
Py-3.8.10	95.11% <100.00%> (+<0.01%)	⬆️
Py-3.8.16	96.62% <100.00%> (+0.12%)	⬆️
Py-3.9.13	95.10% <100.00%> (+<0.01%)	⬆️
Py-3.9.16	96.64% <100.00%> (?)
Py-pypy7.3.11	94.16% <88.88%> (?)
VM-macos	96.48% <100.00%> (?)
VM-ubuntu	96.89% <100.00%> (+0.10%)	⬆️
VM-windows	95.32% <100.00%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
tests/test_cookiejar.py	99.11% <100.00%> (+0.02%)	⬆️
tests/autobahn/test_autobahn.py	98.33% <0.00%> (ø)
tests/test_connector.py	97.88% <0.00%> (+0.13%)	⬆️
tests/test_run_app.py	97.84% <0.00%> (+0.43%)	⬆️
aiohttp/client.py	94.56% <0.00%> (+0.43%)	⬆️
tests/conftest.py	92.98% <0.00%> (+3.50%)	⬆️
tests/autobahn/client/client.py	96.42% <0.00%> (+53.57%)	⬆️

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

[Dreamsorcerer]

Do I understand correctly that the save/load functionality will not be patched to be backward compatible with itself?

I'm not sure exactly how we'd handle this. Possibly adding a version number to the dict or something?
If you'd like to have a go at introducing a backwards compatible approach, please go ahead.

[Dreamsorcerer]

Having said that, it's worth noting that this is a pickle format, so it seems highly unlikely that we'd guarantee that it would never break.
It might break if you save from a newer version of Python and load in an older version (that doesn't support the newer pickle protocol). It's likely to break if we move or change any of the classes involved. It's likely to break if Python changes any of the classes involved (in http.cookies).

So, I don't think we can really provide much of a guarantee here regardless. The fact that the implementation uses pickle suggests to me that it's a convenience function, rather than a file format that should be relied upon from version to version. i.e. I think we should only guarantee that a file saved and loaded from the same machine in the same version of Python and the same x.y version of aiohttp will work.

[Traktormaster]

It sounds like I've not understood truly what the save/load methods provide. The documentation does state it's using a "pickled representation", but I did not think much of it. Maybe a warning at the load would be nice that "Loading a saved value from a different version might not work" or something to make it explicit.

I might look into making a PR later with a save/load serialization option that provides backward compatibility... it seems I have to do it anyways after all.

[patchback]

Backport to 3.9: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 3058c72 on top of patchback/backports/3.9/3058c72dc417cf7fabcc5a1d71e5238f93469012/pr-7221

Backporting merged PR #7221 into master

1. Ensure you have a local repo clone of your fork. Unless you cloned it
   from the upstream, this would be your origin remote.
2. Make sure you have an upstream repo added as a remote too. In these
   instructions you'll refer to it by the name upstream. If you don't
   have it, here's how you can add it:

   $ git remote add upstream https://github.com/aio-libs/aiohttp.git

3. Ensure you have the latest copy of upstream and prepare a branch
   that will hold the backported code:

   $ git fetch upstream
   $ git checkout -b patchback/backports/3.9/3058c72dc417cf7fabcc5a1d71e5238f93469012/pr-7221 upstream/3.9

4. Now, cherry-pick PR Test if CookieJar pickle format breaks #7221 contents into that branch:

   $ git cherry-pick -x 3058c72dc417cf7fabcc5a1d71e5238f93469012

   If it'll yell at you with something like fatal: Commit 3058c72dc417cf7fabcc5a1d71e5238f93469012 is a merge but no -m option was given., add -m 1 as follows instead:

   $ git cherry-pick -m1 -x 3058c72dc417cf7fabcc5a1d71e5238f93469012

5. At this point, you'll probably encounter some merge conflicts. You must
   resolve them in to preserve the patch from PR Test if CookieJar pickle format breaks #7221 as close to the
   original as possible.
6. Push this branch to your fork on GitHub:

   $ git push origin patchback/backports/3.9/3058c72dc417cf7fabcc5a1d71e5238f93469012/pr-7221

7. Create a PR, ensure that the CI is green. If it's not — update it so that
   the tests and any other checks pass. This is it!
   Now relax and wait for the maintainers to process your pull request
   when they have some cycles to do reviews. Don't worry — they'll tell you if
   any improvements are necessary when the time comes!

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

[Dreamsorcerer]

Feel free to change that test alongside any other PRs to improve backwards compatibility.