Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Various cleanups in the federation client code #4031

Merged
merged 2 commits into from
Oct 16, 2018

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Oct 12, 2018

  • Improve logging: log things in the right order, include destination and txids
    in all log lines, don't log successful responses twice

  • Fix the docstring on TransportLayerClient.send_transaction

  • Don't use treq.request, which is overcomplicated for our purposes: just use a
    twisted.web.client.Agent.

  • simplify the logic for setting up the bodyProducer

@richvdh richvdh requested a review from a team October 12, 2018 09:17
- Improve logging: log things in the right order, include destination and txids
  in all log lines, don't log successful responses twice

- Fix the docstring on TransportLayerClient.send_transaction

- Don't use treq.request, which is overcomplicated for our purposes: just use a
  twisted.web.client.Agent.

- simplify the logic for setting up the bodyProducer
@richvdh richvdh force-pushed the rav/cleanup_federation_client branch from b47a851 to 544978b Compare October 12, 2018 09:18
@hawkowl
Copy link
Contributor

hawkowl commented Oct 12, 2018

🤔 all the builds failed

@erikjohnston
Copy link
Member

I take it all back

Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests are angry

@richvdh richvdh dismissed erikjohnston’s stale review October 16, 2018 09:03

the tests are no longer angry

@richvdh richvdh requested a review from a team October 16, 2018 09:03
@richvdh richvdh merged commit b8a5b00 into develop Oct 16, 2018
@richvdh richvdh deleted the rav/cleanup_federation_client branch October 16, 2018 09:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants