CouchDB
  1. CouchDB
  2. COUCHDB-486

Better separation between httpd and core through api layer

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Database Core
    • Labels:
      None
    • Skill Level:
      Committers Level (Medium to Hard)

      Description

      I'm attaching a patch that routes non-purely-functional calls into core CouchDB modules through a new couch_api module. I also went ahead and wrote down dialyzer specs for everything in couch_api. I think this will be a useful reference, will make the codebase a bit more accessible to newcomers, and will help us maintain better separation between the purely functional httpd layer and the core (useful in e.g. partitioning work).

      1. couch_api.patch
        41 kB
        Adam Kocoloski

        Activity

        Hide
        Jan Lehnardt added a comment -

        I'm still undecided on the patch, but I noticed +-compile(export_all) on top. while this is an api module, I'd probably still suggest using explicit exports. but then, I should make up my mind if I like the general idea

        Show
        Jan Lehnardt added a comment - I'm still undecided on the patch, but I noticed +-compile(export_all) on top. while this is an api module, I'd probably still suggest using explicit exports. but then, I should make up my mind if I like the general idea
        Hide
        Adam Kocoloski added a comment -

        Explicit exports are fine by me, the export_all was just a nice shorthand as I was figuring out what needed to be added.

        What do you see as the downsides of the patch, Jan?

        Show
        Adam Kocoloski added a comment - Explicit exports are fine by me, the export_all was just a nice shorthand as I was figuring out what needed to be added. What do you see as the downsides of the patch, Jan?
        Hide
        Adam Kocoloski added a comment -

        forgot to use api for replication. Thanks bitdiddle for checking.

        Show
        Adam Kocoloski added a comment - forgot to use api for replication. Thanks bitdiddle for checking.
        Hide
        Adam Kocoloski added a comment -

        found a few more stragglers, updating the patch

        Show
        Adam Kocoloski added a comment - found a few more stragglers, updating the patch
        Hide
        Damien Katz added a comment -

        I like the ideas behind this patch, but I think I don't like everything dumped into a single module. I think I'd prefer instead to have the same module names and the wrapper calls in the same modules, with the implementation code in a new file. So the public wrapper calls for couch_db would remain in couch_db, but the code is now moved to couch_db_imp or couch_db_priv.

        I think export_all is fine if everything is going to be exported anyway.

        Show
        Damien Katz added a comment - I like the ideas behind this patch, but I think I don't like everything dumped into a single module. I think I'd prefer instead to have the same module names and the wrapper calls in the same modules, with the implementation code in a new file. So the public wrapper calls for couch_db would remain in couch_db, but the code is now moved to couch_db_imp or couch_db_priv. I think export_all is fine if everything is going to be exported anyway.
        Hide
        Adam Kocoloski added a comment -

        That could work. I think it's important in that case to clearly identify the modules that expose the public API; Paul's work to reorganize the source tree could help in that regard..

        Show
        Adam Kocoloski added a comment - That could work. I think it's important in that case to clearly identify the modules that expose the public API; Paul's work to reorganize the source tree could help in that regard..
        Hide
        Paul Joseph Davis added a comment -

        I think Jan has a good point. Though I'm not sure I'd execute it quite like that. If we go through with the source tree reorganization then having a couch_db_client.erl, couch_view_client.erl etc etc is what I've seen in other projects.

        Show
        Paul Joseph Davis added a comment - I think Jan has a good point. Though I'm not sure I'd execute it quite like that. If we go through with the source tree reorganization then having a couch_db_client.erl, couch_view_client.erl etc etc is what I've seen in other projects.
        Hide
        Paul Joseph Davis added a comment -

        s/Jan/Damien/

        Show
        Paul Joseph Davis added a comment - s/Jan/Damien/
        Hide
        Jan Lehnardt added a comment -

        Bump to 1.3.x

        Show
        Jan Lehnardt added a comment - Bump to 1.3.x
        Hide
        Joan Touzet added a comment -

        This is quite old now...I know we'll push this later, but isn't this going to be handled by fabric Real Soon Now?

        Show
        Joan Touzet added a comment - This is quite old now...I know we'll push this later, but isn't this going to be handled by fabric Real Soon Now?

          People

          • Assignee:
            Unassigned
            Reporter:
            Adam Kocoloski
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:

              Development