Opened 7 years ago

Closed 7 years ago

#3022 enhancement closed fixed (fixed)

deprecate twisted.enterprise.util

Reported by: indigo Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: Branch: branches/deprecated-enterprise-3022
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

safe() doesn't actually always make things safe. quote() is horribly misguided in it's attempt to add quotes to some things and not others. All the other components of this module seem to be parts of other deprecated modules. Also, DBAs might burst in flames if they look at the module closely, which is a huge liability.

Change History (10)

comment:1 Changed 7 years ago by glyph

  • Owner changed from glyph to indigo

Pretty much everything in twisted.enterprise (except adbapi) should have large, aggressive warnings in module docstrings explaining they are deprecated, and not deprecated in favor of anything, just nobody should use them ever. Every function in util should, similarly, emit a deprecation warning if you were to call it.

Would you care to contribute a patch which does this?

comment:2 Changed 7 years ago by exarkun

  • Owner changed from indigo to exarkun
  • Status changed from new to assigned

comment:3 Changed 7 years ago by exarkun

  • author set to exarkun
  • Branch set to branches/deprecated-enterprise-3022

(In [22503]) Branching to 'deprecated-enterprise-3022'

comment:4 Changed 7 years ago by exarkun

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

comment:5 Changed 7 years ago by exarkun

  • Priority changed from normal to highest

comment:6 Changed 7 years ago by therve

  • Keywords review removed
  • Owner set to exarkun
  • test_entreprise prints a warning about util when run, this warning should be removed.
  • test_safe and test_getKeyColumn should probably named test_safeDeprecation and test_getKeyColumnDeprecation
  • 2.6.0 could be replaced with 8.0.0, unless I miss something.
  • the warning in util show that the deprecation API is not yet complete. Maybe we could open a ticket for a 'deprecate module' facility.

comment:7 Changed 7 years ago by exarkun

  • Keywords review added
  • Owner exarkun deleted

I renamed the test methods. For the rest, here are my thoughts:

  • twisted.enterprise.util, the module, is deprecated, so there should be a deprecation emitted when it is imported. In some future release, we'll delete the whole file, so code which imports it (even if it doesn't use anything from it) will break, so there should be a warning. I dunno what to do about the warning being emitted by the test suite. It's a problem that shows up in a number of places in Twisted.
  • 2.6.0 could be replaced with 8.0.0, but plenty of other places in Twisted say 2.6.0 now. Since the release person will have to update some of these numbers anyway, I don't think it's a big deal if new changes use 2.6.0 as well.
  • Sounds like a good thing, though I can't think of any way it could work at the moment.

comment:8 Changed 7 years ago by dreid

  • Keywords review removed
  • Owner set to exarkun

I agree about the warning in util.py, but I also don't know how it should work so I don't think it should hold up this ticket.

I do feel like the _unreleasedVersion and _unreleasedDeprecation are a little wonky, but I can't think of a better way to do it. (A module that just holds deprecation functions for twisted versions perhaps?... I don't know.)

Go ahead and merge both of these issues are for the deprecation framework and shouldn't delay deprecation of something as horrendous as t.e.util.

comment:9 Changed 7 years ago by exarkun

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

(In [22677]) Merge deprecated-enterprise-3022

Author: exarkun
Reviewer: therve, dreid
Fixes #3022
Refs #3040

Deprecate twisted.enterprise.util and everything in it.

comment:10 Changed 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.