Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor RestClient to avoid creating circular references #239

Merged
merged 6 commits into from
Oct 25, 2023

Conversation

aaraney
Copy link
Member

@aaraney aaraney commented Oct 25, 2023

See #238 for background.

Previously the RestClient inherited from an AsyncToSerialHelper utility class that provided methods for calling / wrapping sync and async functions synchronously. Due to the way I wrote the AsyncToSerialHelper class, it was possible to add an async function to the event loop that contained a reference to both the AsyncToSerialHelper instance and whatever coroutine was going to run. As a result if the coroutine had a longer lifetime than the AsyncToSerialHelper instance, the gc could never collect the AsyncToSerialHelper instance. Crucially this meant the AsyncToSerialHelper instance's finalizer method, __del__ would never run any tear down logic. To avoid this, the functionality of AsyncToSerialHelper was refactored into a module of functions.

fixes #238

hydrotools._restclient [3.1.0] - 2023-10-25

Removals

  • RestClient.close finalizers were removed as they are no longer necessary.

Changes

  • Loosen _restclient's aiohttp version dependency.
  • Functionality of AsyncToSerialHelper was refactored into a module of functions. To avoid creation of circular references.

Checklist

  • PR has an informative and human-readable title
  • PR is well outlined and documented. See #12 for an example
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (see CONTRIBUTING.md)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output) using numpy docstring formatting
  • Placeholder code is flagged / future todos are captured in comments
  • Reviewers requested with the Reviewers tool ➡️

@aaraney
Copy link
Member Author

aaraney commented Oct 25, 2023

Pinging @jarq6c just so you are aware of this. Im going to go ahead and merge.

@aaraney aaraney merged commit 6c6c033 into NOAA-OWP:main Oct 25, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Determine feasibility of _restclient's continued dependence on aiohttp_cache_client
1 participant