Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.1
    • Fix Version/s: 1.3
    • Component/s: HTTP Interface
    • Labels:
      None
    • Skill Level:
      New Contributors Level (Easy)

      Description

      I'll implement EventSource protocol feed for _changes API (feed="eventsource").

      Some info about it: http://dev.w3.org/html5/eventsource/

      It's more useful than websocket, beacause it's read-only.

        Activity

        Benoit Chesneau made changes -
        Status Open [ 1 ] Closed [ 6 ]
        Fix Version/s 1.3 [ 12318350 ]
        Resolution Fixed [ 1 ]
        Hide
        Benoit Chesneau added a comment -

        Server Sent Events protocol added in 093d2aa6544546a95f6133f1db3c4f4179793f3c

        Show
        Benoit Chesneau added a comment - Server Sent Events protocol added in 093d2aa6544546a95f6133f1db3c4f4179793f3c
        Hide
        Nathan Vander Wilt added a comment -

        I'm happy to report that this thread lives on at https://github.com/indutny/couchdb/commit/a152fb950bff92bc279a81570d96cb77cd71e48b (which is trying to land itself via https://github.com/apache/couchdb/pull/21)

        Show
        Nathan Vander Wilt added a comment - I'm happy to report that this thread lives on at https://github.com/indutny/couchdb/commit/a152fb950bff92bc279a81570d96cb77cd71e48b (which is trying to land itself via https://github.com/apache/couchdb/pull/21 )
        Hide
        Tyler Johnson added a comment -

        Hi Christopher, what is the current status of this patch? Is it abandoned?

        Show
        Tyler Johnson added a comment - Hi Christopher, what is the current status of this patch? Is it abandoned?
        Hide
        Benjamin Young added a comment -
        Show
        Benjamin Young added a comment - Good call, Jan. Done: https://issues.apache.org/jira/browse/COUCHDB-1281
        Hide
        Jan Lehnardt added a comment -

        No objection to adding smart accept handling on top of the feed option, but since it is cumbersome at best and impossible at worst to set accept headers (curl, browsers) I don't think feed= should be deprecated anytime soon.

        I'd say that accept handling here is out of scope of this particular ticket and if that should also be added, someone (hi Benjamin should open another ticket.

        Show
        Jan Lehnardt added a comment - No objection to adding smart accept handling on top of the feed option, but since it is cumbersome at best and impossible at worst to set accept headers (curl, browsers) I don't think feed= should be deprecated anytime soon. I'd say that accept handling here is out of scope of this particular ticket and if that should also be added, someone (hi Benjamin should open another ticket.
        Hide
        Benjamin Young added a comment -

        Personally, I'd prefer to see the "feed" query parameter dropped entirely and have the selection of the feed switch to an Accept header-based (i.e., content negotiation-based) approached. That would allow us to respond correctly to requests for the new text/event-stream content type. A wider ranging change of this kind would of course warrant its own ticket.

        At any rate, using the temporary "experimental-eventsource" thing is fine as long as we support both during the transition to the "eventsource" final option.

        Show
        Benjamin Young added a comment - Personally, I'd prefer to see the "feed" query parameter dropped entirely and have the selection of the feed switch to an Accept header-based (i.e., content negotiation-based) approached. That would allow us to respond correctly to requests for the new text/event-stream content type. A wider ranging change of this kind would of course warrant its own ticket. At any rate, using the temporary "experimental-eventsource" thing is fine as long as we support both during the transition to the "eventsource" final option.
        Hide
        Jan Lehnardt added a comment -

        Christopher, the patch looks clean and nicely done, thanks! Would you consider writing test cases for this in either etap or the JavaScript?

        Everybody, since the EventSource is not yet (as far as I can tell) a final standard, how about naming the feed option value "experimental-evensource" or "eventsource-draft" and rename it to "eventsource" once the spec is final?

        Show
        Jan Lehnardt added a comment - Christopher, the patch looks clean and nicely done, thanks! Would you consider writing test cases for this in either etap or the JavaScript? Everybody, since the EventSource is not yet (as far as I can tell) a final standard, how about naming the feed option value "experimental-evensource" or "eventsource-draft" and rename it to "eventsource" once the spec is final?
        Jan Lehnardt made changes -
        Assignee Benoit Chesneau [ benoitc ] Jan Lehnardt [ janl ]
        Hide
        Dirkjan Ochtman added a comment -

        +1 this would be awesome to have. Would also (mostly?) obviate the need for COUCHDB-841, IMO.

        Show
        Dirkjan Ochtman added a comment - +1 this would be awesome to have. Would also (mostly?) obviate the need for COUCHDB-841 , IMO.
        Christopher Bonhage made changes -
        Hide
        Christopher Bonhage added a comment -

        This patch is based off of the one on Github with some additional cleanup. It also now sends chunks as an io_list instead of appending into a single string.

        EventSource is a great alternative to the tremendous overhead of longpoll. I would love to see this adopted; it seems like a great fit for the _changes feed.

        Show
        Christopher Bonhage added a comment - This patch is based off of the one on Github with some additional cleanup. It also now sends chunks as an io_list instead of appending into a single string. EventSource is a great alternative to the tremendous overhead of longpoll. I would love to see this adopted; it seems like a great fit for the _changes feed.
        Hide
        Fedor Indutny added a comment -
        Show
        Fedor Indutny added a comment - https://github.com/indutny/couchdb/compare/trunk...eventsource-trunk It wasn't deleted, I renamed myself
        Hide
        Benjamin Young added a comment -

        Did anyone happen to snag these patches before that github repo got removed? In future, please submit .patch files here for posterity.

        Also, this should probably use Accept header-based content negation vs. a query param.

        Show
        Benjamin Young added a comment - Did anyone happen to snag these patches before that github repo got removed? In future, please submit .patch files here for posterity. Also, this should probably use Accept header-based content negation vs. a query param.
        Hide
        Jan Lehnardt added a comment -

        Grigory, we've got to support that implementation in the future. Implementing standards that are subject to change adds a maintenance burden. I don't consider it wise to accumulate these burdens with the limited volunteer resources we have.

        Show
        Jan Lehnardt added a comment - Grigory, we've got to support that implementation in the future. Implementing standards that are subject to change adds a maintenance burden. I don't consider it wise to accumulate these burdens with the limited volunteer resources we have.
        Hide
        Fedor Indutny added a comment -

        Glad to see raising interest for my past work.
        If I can fix or change something in that patch - feel free to ask me

        Show
        Fedor Indutny added a comment - Glad to see raising interest for my past work. If I can fix or change something in that patch - feel free to ask me
        Hide
        Grigory V. added a comment -

        That's about web browser developers. Who already did it, ignoring this stupid warning.

        Have you seen the code? It's too simple to have any bugs. It's the same streaming feed with a different Content-Type, "data:" before each line and two line breaks after.

        Show
        Grigory V. added a comment - That's about web browser developers. Who already did it, ignoring this stupid warning. Have you seen the code? It's too simple to have any bugs. It's the same streaming feed with a different Content-Type, "data:" before each line and two line breaks after.
        Hide
        Jan Lehnardt added a comment -

        In addition, the link cites:

        "Implementors should be aware that this specification is not stable. Implementors who are not taking part in the discussions are likely to find the specification changing out from under them in incompatible ways. Vendors interested in implementing this specification before it eventually reaches the Candidate Recommendation stage should join the aforementioned mailing lists and take part in the discussions."

        That's an even better reason for not yet committing this to trunk.

        Show
        Jan Lehnardt added a comment - In addition, the link cites: "Implementors should be aware that this specification is not stable. Implementors who are not taking part in the discussions are likely to find the specification changing out from under them in incompatible ways. Vendors interested in implementing this specification before it eventually reaches the Candidate Recommendation stage should join the aforementioned mailing lists and take part in the discussions." That's an even better reason for not yet committing this to trunk.
        Hide
        Jan Lehnardt added a comment -

        Nobody submitted a review of the proposed patch. We've also been busy with the 1.1.0 and 1.0.3 releases. Developer time is not infinite and this is a volunteer effort.

        If you like to review the proposed patch, we'd like to hear about your results.

        Show
        Jan Lehnardt added a comment - Nobody submitted a review of the proposed patch. We've also been busy with the 1.1.0 and 1.0.3 releases. Developer time is not infinite and this is a volunteer effort. If you like to review the proposed patch, we'd like to hear about your results.
        Hide
        Grigory V. added a comment -

        It's also supported by iOS 4. I wonder why this still isn't added to trunk…

        Show
        Grigory V. added a comment - It's also supported by iOS 4. I wonder why this still isn't added to trunk…
        Show
        Fedor Indutny added a comment - Proposed patch for trunk (Needs testing!): https://github.com/donnerjack13589/couchdb/compare/trunk...eventsource-trunk For 1.0.1 (Tested): https://github.com/donnerjack13589/couchdb/compare/1.0.1...eventsource-1.0.1
        Benoit Chesneau made changes -
        Field Original Value New Value
        Assignee Benoit Chesneau [ benoitc ]
        Fedor Indutny created issue -

          People

          • Assignee:
            Jan Lehnardt
            Reporter:
            Fedor Indutny
          • Votes:
            8 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development