boxo icon indicating copy to clipboard operation
boxo copied to clipboard

session: add new session package

Open Jorropo opened this issue 2 years ago • 2 comments

This replaces #563 and #561.

TODO:

  • [ ] Tests
  • [ ] Wire bitswap to unwrap the session
  • [ ] Revert 22fa8b16a3e530b86542286bebba77c08e17487e and wire the gateway to use this new package.
  • [ ] Changelog (@Jorropo will do later)
  • [ ] ¿ tracing ?

Questions:

  • Right now consumer is a simple comparable to ensure it doesn't panic the map, it might be interesting to enforce something like interface[S any]{ comparable; SessionBadge() S } because this would enforce at compile time that the session consumer is not mixing different session types together (which would not work).
  • Should GetOrCreate return (S, error) instead of S, bool ? Passing errors can always be done efficiently through closures as create does not leak, the important part is to not save in the map failed sessions.
  • Should it be ContextWithSession(context.Context, ...Option) ? We might want to add something like a request root in there.

Jorropo avatar Jan 22 '24 18:01 Jorropo

We ultimately went that route because it satisfy both:

  • Session producers don't need a pointer to some already shared object (like blockservice) to inject a session in their context.
  • It permits session consumers which don't need to maintain more state than in the session object to only inject their session in the context instead maintaining their own uint64 → session structures for it. This allows such state contained sessions to be cleaned up by the GC automatically.

Jorropo avatar Jan 22 '24 18:01 Jorropo

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (4c3a1f2) 65.54% compared to head (fa52546) 65.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #570      +/-   ##
==========================================
- Coverage   65.54%   65.29%   -0.25%     
==========================================
  Files         206      203       -3     
  Lines       25587    25516      -71     
==========================================
- Hits        16770    16660     -110     
- Misses       7344     7372      +28     
- Partials     1473     1484      +11     

see 16 files with indirect coverage changes

codecov[bot] avatar Jan 22 '24 18:01 codecov[bot]