Skip to content
This repository has been archived by the owner on May 25, 2021. It is now read-only.

Handle Etag generation in one place #34

Closed
wants to merge 11 commits into from

Conversation

iilyak
Copy link
Contributor

@iilyak iilyak commented Apr 29, 2015

COUCHDB-2650

@kxepal
Copy link
Member

kxepal commented Apr 29, 2015

Good idea! May be also extract attachment etag generation for consistency?

Rev = couch_doc:rev_to_str({Start, DiskRev}),
<<"\"", Rev/binary, "\"">>.

make_etag() ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a random etag is essentially useless. The only reason cloudant does that (for views) is to preserve the shape of the code relative to couch. Better to omit the etag header entirely if it isn't going to be stable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it was used in CouchDB then? Not because to overcome some cache issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rnewson: I don't think changing how views handle etag generation is in the scope of the bug. I could work on it in the context of another bug if you think it needs to be done before 2.0 release. I would approach this by putting #mrst.sig into meta. Then when we get here, we can use #mrst.sig to calculate etag instead of relying on #vacc.etag (which is uuid at this point).

@iilyak
Copy link
Contributor Author

iilyak commented Apr 29, 2015

Should I call chttpd's version of etag related functions from couch application in cases like. Alternatively I can update couch_chttpd to dispatch the call to chttpd. Which of the above you would prefer?

@kxepal
Copy link
Member

kxepal commented Apr 29, 2015

I think not if only you're going to slowly replace couch_httpd by chttpd. Otherwise, it will make more complicated references between subprojects (they are already quite complicated).

Remove code duplication by using similar function from couch_mrview_show.
show_etag from couch_mrview_show considers query parameteres so we don't
need to pass `lists:usort(chttpd:qs(Req))` in Extra.

COUCHDB-2650
We used to use uuid here. We do proper calculation of Etag now.

COUCHDB-2650
@davisp
Copy link
Member

davisp commented May 21, 2015

This looks mostly good to me. I'm confused by the changes to add header manipulation functions in 9ab5b83 which don't appear to be used anywhere. Is there a corresponding branch in couch_mrview that's not linked here?

@davisp
Copy link
Member

davisp commented May 21, 2015

Ah, found it. Reviewing the others as well.

@iilyak
Copy link
Contributor Author

iilyak commented Sep 12, 2018

stale PR

@iilyak iilyak closed this Sep 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants