Skip to content
Snippets Groups Projects

tor-proto: simplify `ConfluxSet::circuit_action`

Merged opara requested to merge opara/arti:circuit-action into main
3 unresolved threads

This simplifies the call signature of ConfluxSet::circuit_action, and renames it to ConfluxSet::next_circ_action. It now returns a Future instead of a Stream since we only ever use the first item in Reactor::run_once.

@gabi-250 I think these changes make things a bit nicer, but these changes are all opinionated, and I don't feel strongly either way, so feel free to veto them or suggest changes if you don't think they improve things.

/cc @dgoulet

Edited by opara

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
186 197 }
187 198 }
188 199
200 /// Flatten a `Result<Result<T, E>, E>` into a `Result<T, E>`.
201 ///
202 /// See the nightly [`Result::flatten`].
203 fn flatten<T, E>(x: Result<Result<T, E>, E>) -> Result<T, E> {
  • gabi-250
    gabi-250 @gabi-250 started a thread on commit c84ebe1c
  • 176 181 // Note: We don't actually use the returned SinkSendable,
    177 182 // and continue writing to the SometimesUboundedSink in the reactor :(
    178 183 .map(|res| res.map(|res| res.0))
    184 // We only return the first ready action as a Future.
    185 // Can't use `next()` since it borrows the stream.
    186 .into_future()
  • gabi-250
    gabi-250 @gabi-250 started a thread on an outdated change in commit c84ebe1c
  • 537 537 let msg = unwrap_or_shutdown!(self, ret, "control drop")?;
    538 538 Some(CircuitAction::HandleControl(msg))
    539 539 },
    540 res = circ_actions.next().fuse() => {
    541 unwrap_or_shutdown!(self, res, "empty conflux set")???
    542 }
    540 res = circ_actions.fuse() => res?,
  • Thank you, this looks great!

    I have one comment about possibly moving flatten to tor-basic-utils, but it's non-blocking. Feel free to merge when you're ready.

  • gabi-250 approved this merge request

    approved this merge request

  • David Goulet approved this merge request

    approved this merge request

  • opara added 1 commit

    added 1 commit

    • 49c02757 - tor-basic-utils: move `flatten` from tor-proto

    Compare with previous version

  • opara enabled an automatic merge when all merge checks for 49c02757 pass

    enabled an automatic merge when all merge checks for 49c02757 pass

  • opara mentioned in commit 8cf4c826

    mentioned in commit 8cf4c826

  • merged

  • Please register or sign in to reply
    Loading