Enable use of a custom PortalClient

[ntBre]

Description

This PR allows users to supply a custom PortalClient constructor to functions that use a client internally (most notably *ResultCollection.to_records) by overriding the global openff.qcsubmit.utils.utils.default_portal_client function, or through the use of the new portal_client_manager context manager. This is especially useful for users needing to supply a keyword like verify=False (#282) or for users wanting to take advantage of the caching in QCPortal 0.54.

Example usage

from qcportal import PortalClient
from openff.qcsubmit.utils.utils import portal_client_manager

with portal_client_manager(lambda addr: PortalClient(addr, cache_dir=".")):
    ...

I think an added benefit of this approach is potentially making it easier to mock PortalClient for tests. I worked on an example of this for a little while, but you have to implement a little more functionality than I expected initially, at least to call something like to_records. Still, I think it's doable and potentially useful.

Todos

• [x] Rough draft of interface
• [ ] Docs
• [ ] Better test(s)?
• [ ] Add to examples

Questions

• [ ] What should the API look like, and where should the code live?

Status

• [ ] Ready to go

[ntBre]

Thanks for the review! I've made default_portal_client private, added a context manager for the socket trick, and added a second test for the portal_client_manager unimaginatively named test_manager2. I think this one probably supersedes the old test, so I can delete the original test_manager and the dataset I added and rename this one if you also prefer this one.

Should portal_client_manager be re-exported in utils/__init__.py so that it doesn't have to be imported from openff.qcsubmit.utils.utils? I can also move the no_internet context manager (and/or rename it), but I wasn't sure if that was better to do now or later when another test needed it.

[ntBre]

I added a docstring to the portal_client_manager, but I noticed in the docs build that it is not displayed anywhere. This is possibly outside the scope of this PR, but it might be nice to update the doc generation to include this. There is also still a Caching section in the generated docs that points to code that doesn't exist anymore.

Anyway, if the new test is good enough, I think that resolves the Todos besides possibly adding this to the Examples. I included an example usage in the docstring, but I can also add this to an example notebook if you want.

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[ntBre]

I think I handled all of the reviews! I also rebased onto main and force pushed here, not realizing that that might complicate the reviews. Sorry in advance if that's the case.

I think the only change I made outside of your suggestions was re-exporting the portal_client_manager from utils.utils to just utils. Hopefully that's okay. I also went ahead and updated the release notes so that this will be ready to merge when all the reviews are resolved.

I think the docs look pretty good, but from a quick search, I think I might need double backquotes to get the inline code to render as I expected instead of the single backquotes I used. The parameter section also seems a little off. I'm happy to tweak both of those if you want, but I figured I would comment here while the checks have passed before I start pushing more docstring changes.

[ntBre]

I fixed the issues I saw in the docs, should be ready for another review!