Opened 2 years ago

Closed 20 months ago

#6026 enhancement closed fixed (fixed)

Port twisted.protocols.basic.FileSender to Python 3

Reported by: itamar Owned by: tom.prince
Priority: normal Milestone: Python-3.x
Component: core Keywords:
Cc: Branch: branches/filesender-py3-6026-2
(diff, github, buildbot, log)
Author: therve Launchpad Bug:

Description

FileSender is not being ported to Python 3 as part of #6025, so will need to be done separately.

Change History (13)

comment:1 Changed 23 months ago by therve

  • Owner set to therve

comment:2 Changed 23 months ago by therve

  • Author set to therve
  • Branch set to branches/filesender-py3-6026

(In [36403]) Branching to 'filesender-py3-6026'

comment:3 Changed 23 months ago by therve

  • Keywords review added
  • Owner therve deleted

This is ready for review. This was mainly about adding new tests, as there was no incompatible bits that I could spot. I did change the interface implemented and remove one method not part of the interface, though.

comment:4 Changed 21 months ago by tom.prince

comment:5 Changed 21 months ago by glyph

I clicked on the given link but I didn't actually see build results. Is there a buildbot bug here someplace, or is 4 days actually so long that our build history falls off the end?

comment:6 Changed 21 months ago by tom.prince

Typo in the link: here

comment:7 Changed 20 months ago by tom.prince

  • Owner set to therve
  1. I don't think removing pauseProducing without deprecation is allowable, under the compatibility policy.
  2. Is there any reason to use a real file, rather than something like StringIO? Also, the consumer should probably be something designed for testing like StringTransport.
  3. New tests should be synchronous when possible, using successResultOf, failureResultOf, and assertNoResult.
  4. The tests don't test the case when multiple chunks are required to send the file. (It is probably reasonable to shrink CHUNK_SIZE when testing this.

Please resubmit for review, after addressing the above points.

comment:8 Changed 20 months ago by tom.prince

  • Keywords review removed

comment:9 Changed 20 months ago by therve

  • Branch changed from branches/filesender-py3-6026 to branches/filesender-py3-6026-2

(In [37088]) Branching to 'filesender-py3-6026-2'

comment:10 Changed 20 months ago by therve

  • Keywords review added
  • Owner therve deleted

Thanks, everything handled I think!

comment:11 Changed 20 months ago by tom.prince

  • Keywords review removed
  1. The comments in the test should explain why resumeProducing needs to be called twice.
  2. test_transferMultipleChunks doesn't actually test that things are sent in chunk-size bits. It should check the output at each step.
  3. Something should verify that the producer gets unregistered.
  4. Similarly, something should verify that the producer gets properly registered as a non-streaming producer.

I'll fix these up and commit.

comment:12 Changed 20 months ago by tom.prince

  • Owner set to tom.prince

Also, stopProducing isn't normally called, except in exception cases, so don't call it from the tests except in exceptional cases.

comment:13 Changed 20 months ago by tomprince

  • Resolution set to fixed
  • Status changed from new to closed

(In [37101]) Merge filesender-py3-6026-2: Port twisted.protocols.basic.FileSender to Python 3.

Author: therve
Reviewers: tom.prince
Fixes: #6026

The interface implemented changed to the more specific IPullProducer.

Note: See TracTickets for help on using tickets.