Return written path in Blob.write_to_path

[pquentin]

I'm in the process of replacing all urllib3 certs with certs generated by trustme. To make the transition less painful, I'm keeping the file-based interface. See urllib3/urllib3@672eaab, urllib3/urllib3@a9776d1, urllib3/urllib3@5fa4531, urllib3/urllib3@84abc7f, urllib3/urllib3@4903840 and urllib3/urllib3@62ef68e for the work done so far.

The general pattern is:

cls.path = os.path.join(cls.certs_dir, "path.pem")
ca.cert_pem.write_to_path(cls.path)
# now we can use cls.path

I plan to switch to pathlib in the future, but I think it would be less verbose to just write cls.path = ca.cert_pem.write_to_path(os.path.join(cls.certs_dir, "path.pem")) instead, hence this patch.

[codecov]

Codecov Report

Merging #152 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #152   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines         385    385           
  Branches       25     25           
=====================================
  Hits          385    385

Impacted Files	Coverage Δ
tests/test_trustme.py	100% <100%> (ø)	⬆️
trustme/__init__.py	100% <100%> (ø)	⬆️

[sethmlarson]

This seems fine to me because write_to_path() probably doesn't need to return anything else. Although I wonder about having a function always return it's only parameter? That seems strange to me. 🤷‍♂

[njsmith]

I guess this is useless on py 3.8+, because if you really want to put everything on one line you can write:

ca.cert_pem.write_to_path(cls.path := os.path.join(cls.certs_dir, "path.pem"))

Jamming that many operations into one line isn't my favorite thing, and returning the argument doesn't seem terribly idiomatic. Also, I wonder if for the urllib3 stuff you'd be better served with some even higher-level convenience helpers? E.g.

ca_cert_path, server_key_path, server_cert_path = write_certs(tmpdir, ca, server_cert)

But I'm not hugely opposed either; if you want to push this then I'll probably give in :-)

[pquentin]

The idea would be to write this eventually:

cls.path = ca.cert_pem.write_to_path(cls.certs_dir / "path.pem")

Which is maybe not that bad? The problem with a write_certs helper is that it turns out that there's a lot of variety: certs with the whole chain, certs that are combined with their keys and cases where I only want to write the CA cert. But well, that's two reviews telling that what I'm proposing is weird/not idiomatic, so I'm going to close this for now! And I'll finish the migration first.

Thank you for the reviews! Always appreciated.