Ticket #6026 enhancement closed fixed

Opened 8 months ago

Last modified 3 months ago

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
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

1

Changed 6 months ago by therve

  • owner set to therve

2

Changed 6 months ago by therve

  • branch set to branches/filesender-py3-6026
  • branch_author set to therve

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

3

Changed 6 months ago by therve

  • owner therve deleted
  • keywords review added

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.

4

Changed 5 months ago by tom.prince

5

Changed 5 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?

6

Changed 5 months ago by tom.prince

Typo in the link:  here

7

Changed 3 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.

8

Changed 3 months ago by tom.prince

  • keywords review removed

9

Changed 3 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'

10

Changed 3 months ago by therve

  • owner therve deleted
  • keywords review added

Thanks, everything handled I think!

11

Changed 3 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.

12

Changed 3 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.

13

Changed 3 months ago by tomprince

  • status changed from new to closed
  • resolution set to fixed

(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.