-
Notifications
You must be signed in to change notification settings - Fork 45
Normalize special CouchDB headers names #15
base: master
Are you sure you want to change the base?
Conversation
COUCHDB-2353
If the solution is ok, I'll open few more PRs against chttpd, replicator and mrview. |
This obliviously breaks backward compatibility, but 2.0 is a good time to cleanup and normalize stuff. |
Do we want to worry about backwards compatibility here or not? Or maybe we can add a check that throws a 400 if it detects an old header name? This just feels like one of those places where people will get frustrated when something doesn't work like it used to without any feedback or indication of what exactly changed. |
@davisp I found myself more frustrated because special headers are don't follows some single logic and I have always consult myself with docs to be sure that I name it right (since CouchDB will just ignore unknown header). As for backward compatibility point, it only matters for replicator which uses X-Couch-Full-Commit to force CouchDB accept new docs with delayed_commits on. I believe, this isn't what we want to let him doing, but that's another story. |
@kxepal Sure, I agree that normalizing things to be consistent is a good idea. I'm just saying that for any user that happens to miss this particular change in the log will have a very bad experience figuring the change out. If we instead detected the presence of old headers and returned a 400 Bad Request we could inform them directly that the header names have changed. Alternatively we could upgrade old header names though in the interest of deprecating I'd prefer the 400 I think. |
Can we please stop the meme of “we are bumping the major version so let’s break everything.”? We should be very deliberate with what we break and when, in order to avoid a Python 3 situation. From my post on dev@ earlier today on the
I do like the idea of detecting the “wrong” case and logging an error so people have a chance to upgrade it. Maybe also send an x-couch-deprecation-notice header with an URL to the docs where it is explained. -1 on breaking BC for this without a plan. (sorry @kxepal :) |
We'd already discussed this moment: nobody will look and process this header to be notified about.
Question not about breaking everything in the name of major version bump, but cleanup existed things. What you suggests ( And I just proposed this change to trigger the discussion about how handle it right (: @davisp suggested good idea with 400 response, so users will be alerted about using old stuff for new environment. More ideas are welcome (: |
Then maybe not :) — I know I’d be looking at it, but that’s not representative.
I’ve only seen the latter work satisfactory. But that’s my personal bubble :) Thanks for getting the discussion rolling! I think at some point we generally agreed on this process:
So in this case, I’d say:
|
Good idea and well balanced. I like it (: Will rewrite my PR to match it. |
COUCHDB-2353