Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary crud from werkzeug.contrib #4

Closed
mitsuhiko opened this issue Oct 18, 2010 · 23 comments · Fixed by #1442
Closed

Remove unnecessary crud from werkzeug.contrib #4

mitsuhiko opened this issue Oct 18, 2010 · 23 comments · Fixed by #1442
Milestone

Comments

@mitsuhiko
Copy link
Contributor

Some things ended up in contrib that really shouldn't have been there.

@EnTeQuAk
Copy link
Contributor

EnTeQuAk commented Jan 3, 2011

The question is what do we want to be werkzeug.contrib? Should it only contain wsgi related stuff or should it contain helpers for web development?

If it's the first one (only wsgi-related helpers) then werkzeug.contrib.cache and contrib.atom should go away.

Another question: is iterio even used widely, what was it's purpose? It depends on greenlet and seems to be the only part that does not even work without a external dependency.

@mitsuhiko
Copy link
Contributor Author

iterio should go into a separate PyPI package. Maybe the same should be done for cache, not sure yet. The modules from contrib I use are securecookie and cache. I don't think I ever used anything else from there. We could do a poll and ask what people are using enough that it's worth having in Werkzeug itself.

Cache however lacks support for newer memcache adapters, so if it sticks, it needs an updated interface.

untitaker pushed a commit to untitaker/werkzeug that referenced this issue Mar 16, 2013
Fixed remaining iterating-related stuff.
DasIch added a commit that referenced this issue May 26, 2013
@untitaker
Copy link
Contributor

Cache is pretty well-maintained now, but i'd like to get it into a separate pkg anyway. The only module which really should stay is fixers IMO.

@untitaker untitaker added this to the 1.0 milestone Aug 24, 2014
@untitaker
Copy link
Contributor

I wonder if we should move even more modules into separate packages, even though they are directly relevant to Werkzeug.

An example would be werkzeug.urls: A general-purpose backport of urllib.parse is extremely useful outside of Werkzeug's context, and it'd be very useful to have potentially more maintainers on it who are otherwise not interested in using Flask or Werkzeug.

@RonnyPfannschmidt
Copy link
Contributor

i like the idea, but it needs some preparation and thougts

@asteinlein
Copy link

I could be interested in helping out here, but would obviously need some decisions being made here. One idea would be extract some parts to external packages, i.e. werkzeug.contrib.cache to werkzeug-cache or some such, middlewares to werkzeug-middleware, etc.

We could for instance include these packages as dependencies in Werkzeug for a while and alias them in their existing locations for backwards compatibility, while emitting deprecation warnings about their future removals.

@RonnyPfannschmidt
Copy link
Contributor

sounds like a good plan to me, @mitsuhiko @untitaker do you agree?

in case the others agree i propose providing @asteinlein with a repo under pallets for werkzeug-cache as a starting point and using werkzeug_cache as potential module name

@untitaker
Copy link
Contributor

Agreed! Will set up a repo later.

IIRC somebody already forked Werkzeug's profiler middleware into a new package.
We should seek collaboration from that person.

On Fri, May 27, 2016 at 01:59:58AM -0700, Ronny Pfannschmidt wrote:

sounds like a good plan to me, @mitsuhiko @untitaker do you agree?

in case the others agree i propose providing @asteinlein with a repo under pallets for werkzeug-cache as a starting point and using werkzeug_cache as potential module name


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#4 (comment)

@asteinlein
Copy link

asteinlein commented May 28, 2016 via email

@untitaker
Copy link
Contributor

untitaker commented May 28, 2016

Middlewares will require more discussion, which ones to keep in Werkzeug and which not.

The things that IMO definetly can be factored out are:

  • cache
  • atom
  • fixers
  • profiler

@untitaker
Copy link
Contributor

Let's start with cache: https://github.com/pallets/werkzeug-cache

@asteinlein I've added you as a collaborator to werkzeug-cache, @RonnyPfannschmidt you're a member anyway.

@untitaker untitaker self-assigned this May 29, 2016
@untitaker
Copy link
Contributor

Oh, I forgot that werkzeug.locals is a candidate as well!

@davidism
Copy link
Member

davidism commented Jun 5, 2016

werkzeug.contrib.wrappers contains some interesting request/response mixins. At least ProtobufRequestMixin should probably go. The ones we keep should move into werkzeug.wrappers. I'm already doing this for JSONRequestMixin. (It might be a good idea to split wrappers into two moduels, request and response.)

@mitsuhiko
Copy link
Contributor Author

+1 on these:

  • move local into a repo
  • move cache into a repo
  • find a solution for contrib wrappers

-1 on urls

Two reasons for this: a) i don't want to start having tiny dependencies everywhere. This is a huge maintenance hurdle and I keep learning this over and over at Sentry. Secondly because it's not even an entirely correct implementation of URLs.

@untitaker
Copy link
Contributor

@mitsuhiko I was proposing urls because I once needed something that behaves the same on both Py2 and Py3 and didn't want to depend on a WSGI library for that.

@lepture
Copy link
Contributor

lepture commented Feb 4, 2018

@untitaker why is the repo (https://github.com/pallets/werkzeug-cache) deleted?

@untitaker
Copy link
Contributor

@lepture i cannot remember but i think the transition was not properly coordinated, people still submitted PRs

@lepture
Copy link
Contributor

lepture commented Feb 4, 2018

@untitaker I'm closing issues related to cache. #1249

I'll make a separated repo in next week.

@lepture
Copy link
Contributor

lepture commented Nov 28, 2018

Created https://github.com/pallets/cachelib

@davidism
Copy link
Member

davidism commented Dec 7, 2018

I added cards for all the contrib code to https://github.com/pallets/werkzeug/projects/2

@davidism
Copy link
Member

davidism commented Dec 7, 2018

I didn't add a card for werkzeug.local because it's not in contrib, although we may still want to extract it eventually.

@davidism
Copy link
Member

davidism commented Dec 7, 2018

Things I marked as "discuss, probably remove":

  • wrappers.RoutingArgsRequestMixin - seems to be a spec that was never adopted, although it almost matches Flask, which sets Request.view_args during route matching.
  • wrappers.ReverseSlashBehaviorRequestMixin - not sure when this would be used, seems like other code in Werkzeug is handling leading and trailing slashes
  • fixers.PathInfoFromRequestUriFix - is this still a problem with Windows? Is CGI on Windows Apache something we need to keep a fixer for?
  • fixers.HeaderRewriterFix - not sure if this is worth maintaining. It's pretty straightforward for an app to implement exactly what they need, with more nuance than this gives. The examples (removing Date, adding X-Powered-By`) both seem to be bad practice (mandatory header, fingerprinting information).

If we're +/-0 on any of these, we should prefer adding a deprecation notice. If people report they're still using something, we can always reconsider the deprecation before 1.0.

@davidism davidism modified the milestones: 1.0, 0.15 Jan 12, 2019
@davidism
Copy link
Member

Marking this for 0.15 to deprecate everything that needs to be. We can have a separate task to remove deprecated code for 1.0, which will include more than this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
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 a pull request may close this issue.

7 participants