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

Modular output via plugins #9

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

timwis
Copy link

@timwis timwis commented Aug 5, 2015

Stemming from esri/koop#195, this pull request proposes the following path for queries

  • provider -> router -> controller.findResource
    • interface.parse
      • parse SELECT and WHERE params into some common form (SQL string, ORM, or JS object)
    • model.getResource
      • cache.get
        • db.select
          • same logic as before but using the common form described above
          • also parse select/outfields here
    • interface.formatOutput
      • convert GeoJSON to expected output of interface

NOTE this pull request depends on pull koopjs/koop#226

@ungoldman
Copy link
Contributor

Your implementation makes sense to me, but tests are not passing. I'm going to pull this down and refactor a bit.

@ungoldman
Copy link
Contributor

Wow, realizing this module is really out of date. I'll refactor and incorporate your changes. Thanks @timwis for bringing attention to this.

@timwis
Copy link
Author

timwis commented Aug 7, 2015

Interesting... which module is the most up-to-date? Curious how the new ones work / direction it's going

@ungoldman
Copy link
Contributor

@timwis: @dmfenton is actively working on refactoring koop-agol, and I believe koop-socrata is the most robust and up to date as of right now. We're actively working on improving patterns and maturing the project. @chelm has basically been solo on all of these modules for a long time (a monumental task), and we've only recently devoted more resources to be able to come in to help out and make the project more usable for outside use cases (for folks like you). We're running koop & koop-agol in production for datasets hosted through ArcGIS Open Data.

@ungoldman ungoldman mentioned this pull request Aug 8, 2015
@jgravois
Copy link

jgravois commented Feb 8, 2016

@ngoldman thank you so much for refactoring and including tests in #10. it doesn't look like you cherry picked any of @timwis' commits, but i'm guessing you incorporated the helpful work yourself.

is this correct? does that mean this PR is safe to close?

@ungoldman
Copy link
Contributor

@jgravois nothing from here is incorporated as far as I know @jgravois. that PR was from a long time ago :(

@jgravois
Copy link

jgravois commented Feb 8, 2016

nothing from here is incorporated as far as I know

no worries. i'll try and take a look soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants