Opened 5 years ago

Closed 3 years ago

#4111 task closed fixed (fixed)

Remove deprecated twisted.enterprise.util

Reported by: thijs Owned by: itamar
Priority: normal Milestone:
Component: core Keywords:
Cc: itamarst, thijs, itamar Branch: branches/enterprise-util-4111-2
(diff, github, buildbot, log)
Author: thijs, itamarst Launchpad Bug:

Description

trunk/twisted/enterprise/util.py is deprecated, let's remove it.

Change History (25)

comment:1 Changed 5 years ago by thijs

  • Owner changed from glyph to thijs
  • Status changed from new to assigned

#4108, #4109, #4110 all depend on this module so this should become a master ticket.

comment:2 Changed 5 years ago by thijs

  • Author set to thijs
  • Branch set to branches/enterprise-util-4111

(In [28082]) Branching to 'enterprise-util-4111'

comment:3 Changed 5 years ago by thijs

(In [28086]) Remove deprecated t.enterprise.util and related modules, refs #4111

comment:4 Changed 5 years ago by thijs

  • Keywords review added
  • Owner thijs deleted
  • Status changed from assigned to new

comment:5 Changed 4 years ago by khorn

  • Owner set to khorn

comment:6 Changed 4 years ago by khorn

  • Keywords review removed
  • Owner changed from khorn to thijs

Looks good. Please merge.

comment:7 Changed 4 years ago by exarkun

Missing a news file.

comment:8 Changed 4 years ago by thijs

(In [28114]) Remove references to t.enterprise.row, refs #4111

comment:9 Changed 4 years ago by thijs

(In [28115]) Remove row example, refs #4111

comment:10 Changed 4 years ago by thijs

  • Keywords review added
  • Owner thijs deleted

I removed some more row stuff. twisted/persisted/journal/rowjournal.py refers to t.e.row but doesnt import it. what to do with this module, deprecate it and remove the reference to t.e.row?

comment:11 follow-up: Changed 4 years ago by exarkun

  • Keywords review removed
  • Owner set to thijs
  1. Please delete all of test_enterprise in test_import.py. These tests are almost all obsolete and we should be moving towards eliminating them completely.
  2. About rowjournal: yes, please deprecate it and remove the reference. sigh. Bug itamar about what he thinks about deprecating the rest of the journal package, too.

Thanks

comment:12 Changed 4 years ago by thijs

  • Cc itamarst added
  • Status changed from new to assigned

comment:13 Changed 4 years ago by thijs

(In [28433]) Delete obsolete test_enterprise tests, refs #4111

comment:14 Changed 4 years ago by thijs

(In [28437]) Deprecate twisted.persisted.journal.rowjournal. Also apply some coding standards. refs #4111

comment:15 Changed 4 years ago by thijs

  • Keywords review added
  • Owner thijs deleted
  • Status changed from assigned to new

I deprecated the rowjournal module, but not sure if I did it correctly, but based on this session it seems I have:

>>> import twisted.persisted.journal.rowjournal
__main__:1: DeprecationWarning: twisted.persisted.journal.rowjournal is deprecated as of Twisted 10.1
>>> from twisted.persisted.journal.rowjournal import INSERT
__main__:1: DeprecationWarning: twisted.persisted.journal.rowjournal is deprecated as of Twisted 10.1
>>> from twisted.persisted.journal.rowjournal import RowJournal
__main__:1: DeprecationWarning: twisted.persisted.journal.rowjournal is deprecated as of Twisted 10.1

The CompatibilityPolicy doesn't describe how to deprecate a module or class, as far as I can see, so if someone could add a proper description that to page, great.

Up for review.

comment:16 in reply to: ↑ 11 Changed 4 years ago by thijs

  • Cc itamar added

Replying to exarkun:

Bug itamar about what he thinks about deprecating the rest of the journal package, too.

I've opened #4298 for this and assigned it to itamar.

comment:17 Changed 4 years ago by TimAllen

  • Owner set to TimAllen

comment:18 Changed 4 years ago by TimAllen

  • Keywords review removed
  • Owner changed from TimAllen to thijs

I've reviewed r28437 of the branch.

  1. I note that t.e.adbapi is no longer being imported from twisted/test/test_import.py even though it isn't deprecated - is that deliberate?
  2. As exarkun noted in comment:7, it's still missing a news file (see the relevant section of the ReviewProcess page).
  3. The CompatibilityPolicy page says "Like all changes in Twisted, deprecations must be unit tested." There should be a deprecation test added to the RowJournal tests.

comment:19 Changed 3 years ago by <automation>

  • Owner thijs deleted

comment:20 Changed 3 years ago by itamar

  • Owner set to itamar

comment:21 Changed 3 years ago by itamarst

  • Author changed from thijs to thijs, itamarst
  • Branch changed from branches/enterprise-util-4111 to branches/enterprise-util-4111-2

(In [33325]) Branching to 'enterprise-util-4111-2'

comment:22 Changed 3 years ago by itamar

  1. test_import is bogus, and apparently there's a ticket to get rid of it, so I'm not going to bother readding adbapi.
  2. Fixed.
  3. I just deleted rowjournal. Having a deprecation warning for a module that is completely broken and unusable because we deleted its dependencies doesn't make sense.

I'll submit for review after final test run.

comment:23 Changed 3 years ago by itamar

  • Keywords review added
  • Owner itamar deleted

OK, ready for review (in case you're a new reviewer, note that the branch also covers ticket #4109).

http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/enterprise-util-4111-2

comment:24 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner set to itamar

In addition to #4109, this seems to cover #4108 and #4110 as well, right?

The news fragment for all affected tickets should be in .removal files.

The twisted.enterprise.adbapi.safe deprecation is perhaps somewhat related to this change. We could maybe get rid of it now as well.

Please merge once safe is gone, or at least file a new ticket for removing it.

comment:25 Changed 3 years ago by itamarst

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

(In [33412]) Merge enterise-util-4111-2.

Author: thijs, itamar
Review: khorn, exarkun, TimAllen
Fixes: #4108, #4109, #4110, #4111

Remove deprecated code from the enterprise package, along with relevant documentation and examples.

Note: See TracTickets for help on using tickets.