Skip to content

Conversation

@AstraLuma
Copy link

Summary

Adds the ability for containers to have sub-containers. (Previously floated as #70.)

Pull Request Check List

  • Typos aside (please, always submit typo fixes!), I understand that this pull request may be closed in case there was no previous discussion.
  • Do not open pull requests from your main branch – use a separate branch!
    • There's a ton of footguns waiting if you don't heed this warning. You can still go back to your project, create a branch from your main branch, push it, and open the pull request from the new branch.
    • This is not a pre-requisite for your pull request to be accepted, but you have been warned.
  • Added tests for changed code.
    • The CI fails with less than 100% coverage.
  • New APIs are added to our typing tests at https://github.com/hynek/svcs/blob/main/tests/typing/.
  • Updated documentation for changed code.
    • New functions/classes have to be added to docs/core-concepts.md or one of the integration guides by hand.
    • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
      • The next version is the second number in the current release + 1. The first number represents the current year. So if the current version on PyPI is 23.1.0, the next version is gonna be 23.2.0. If the next version is the first in the new year, it'll be 24.1.0.
  • Documentation in .rst and .md files is written using semantic newlines.
  • Changes (and possible deprecations) are documented in the changelog.
  • Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.

We can't just ask the parent for the value, since that'll just cause the
parent to instantiate it.

Instead, Container has to directly walk its ancestors and do the work
itself.
@AstraLuma
Copy link
Author

AstraLuma commented Oct 17, 2025

This is me attempting to upstream some forkish work (ie, making it less of a fork).

I'm assuming that people are going to have Opinions:tm: about this.

@hynek
Copy link
Owner

hynek commented Oct 20, 2025

I'm up to my ears in trying to get structlog out, but could you outline how your approach is supposed to work so I don't have to deduce it from the code? :)

@AstraLuma
Copy link
Author

Ok, so

The thought is that containers are meant to match the various lifecycles of a complex app. eg, a complex FastAPI app has the overall app, request contexts, spawned tasks, queued jobs, etc. In particular, FastAPI has tasks that spawn from a request (and presumably should inherit the context and therefore the container).

The implemented behavior:

  • All old code still works (minimal changes in the existing test suite)
  • Containers can now have parents
  • The parent-child link is preserved until both containers are unused--the parent doesn't close until all of its children have closed
  • Context managers are how "usage" is tracked
  • Containers are re-entrant context managers
  • The container doesn't close until the last context manager exits
  • Instances in the parent container are used by the child
  • This happens if the parent's instance is created after the child is spawned
  • Local registries still work and are inherited
  • Instances anywhere in the ancestry list override all registries

Additionally, an inheritable flag is implemented. This is meant to be used to prevent eg database transactions from spanning contexts. I'm not convinced I got the semmantics exactly right, though.

  • Non-inheritable (unheritable) instances are not used by child containers
  • Local registry inheritance still happens
  • They are otherwise identical to inherited services

@AstraLuma
Copy link
Author

AstraLuma commented Oct 20, 2025

This above explanation glosses over a subtlety that i think produces a warty behavior. Non-inheritable services still close when the container closes--when all children are unused and close. This is significantly later than before, which can cause problems if you depend on when services close.

For example, you have the current database transaction in a FastAPI app making use of spawned tasks. You largely want the spawned task to inherit the context--the request that triggered it, the current user, external services, etc. But you might want a fresh transaction.

The trick here is that with the naive implementation, the request's direct transaction won't commit until the spawned task is done. I think most people would consider this an undesirable behavior.

This feels a little bit cursed. I want to keep the model as simple as possible, but the only solutions I can think of involve more complexity. Some examples:

  • Add a half-closed state to containers--containers will close non-inheritable services when direct usages are done, but inheritable services wait until all children are done
  • Add a fork callback to services--services have the option to construct a derived copy for children (possibly combined with the above)
  • Linked parent-children was a mistake, and maybe unlinked copies or something is more correct
  • Half-way, where when a parent is closed, children get copies of inheritable services
  • Secret fifth thing

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.

2 participants