koopjs / koop-pgcache

PostGIS cache for Koop.
Other
4 stars 4 forks source link

Host field too small, increased size GET #26

Closed PeaceNlove closed 8 years ago

PeaceNlove commented 8 years ago

The host field contains most of the time URLs, therefore the field should be able to contain urls with a length > 100 char, because browsers and webservers limit URLs to 2KB (this may vary). In a custom provider I'm working on, I use the host field to store even more data about this host in json, and this exceeds 100 chars as well.

dmfenton commented 8 years ago

Can you share an example of what you're trying to store in this field?

PeaceNlove commented 8 years ago

I have two examples: 1: In this case, I want to retrieve geojson from a server object extension on Arcgis Server, the following url isn't available online, but illustrates how urls to Server object extensions tend to get very long: http://webapplicatie-ont-proj.intern.agro.nl/arcgisiis/rest/services/Geogegevensbeheer/beheer_regelingspercelen_bbr/MapServer/exts/Situatie/TijdlijnCorrectieVoorstel

2: in het hosts field, I'm storing a little bit more about this host (which is a OGC WFS) to make sure I have all necessary parameters later on when I fetch the data. I put all this together in a JSON string and the JSON propertynames and other JSON overhead make this string very quickly go over 100 chars For example {"host": "http://geodata.nationaalgeoregister.nl/inspire/ad/wfs?", "version": "1.0.0", "filter" :" POPULATION1000"} Filter is a WFS filter and sometimes necessary to limit the amount of features from the WFS, the default behaviour of my new WFS provider is to page over all results, but when the WFS contains a very large number of features (e.g. all buildings in the Netherlands), you really don't want these in memory. All of this additional WFS information will eventually end up in a GET request to the WFS, so in my opinion, it makes sense to store this information directly in a url in the host field or via a json string in the host field and therefore I proposed this change for the length of host field

ungoldman commented 8 years ago

Though I can't say I'd recommend storing a completely different type in the host field than what is expected for your second use case, I don't see any problem with increasing the size of the host field to help deal with the use case in your first example.

I reran the tests, they were only failing because of the off-by-one error which @dmfenton fixed in #30.

@dmfenton is this alright to merge? Needs a change log addition but otherwise don't see this negatively effecting anything.

dmfenton commented 8 years ago

I think it makes more sense just to modify your database locally versus changing this for all users when we're not sure how applicable this is to others.

@PeaceNlove do you know how to do that?

ungoldman commented 8 years ago

@dmfenton the maximum length of a URL is 2083 characters. If host is meant for URLs, and considering ArcGIS Server URLs (a primary use case for Koop) are often very long, shouldn't we support something closer to the standard maximum URL length than the rather limiting 100 for all users?

dmfenton commented 8 years ago

To be honest, I don't understand the performance impact well enough to make a good call here. @JerrySievert do you have anything to offer?

JerrySievert commented 8 years ago

to be honest, when dealing with nebulous things like URLs and needing to store in databases, i tend to use a straight VARCHAR without a size. on modern DBMSs (such as Postgres), this isn't a glaring performance hit.

how many different backing stores are we talking about?

dmfenton commented 8 years ago

It's just Postgres and Elasticsearch for now

JerrySievert commented 8 years ago

i'd recommend dropping the size on Postgres:

this._createTable(type, '(id varchar(100), host varchar)', null, function (err, result) {

and using a simple string type on elastic search.

dmfenton commented 8 years ago

Thanks for the wisdom @JerrySievert! @PeaceNlove you want to modify this PR to implement that?

PeaceNlove commented 8 years ago

Thanks for having a look at this. I think I modified the PR with the recommendation from Jerry, but I'm a Github newbie, so I'm not sure whether I've done this right. Can one of you check this?

By the way, I will do a presentation on Koop at the Dutch Esri Gis Conferentie. It will be the second part of a presentation by Groningen University, the first part will be about the Esri open data portal.

ungoldman commented 8 years ago

This looks good to me, merging.

ungoldman commented 8 years ago

By the way @PeaceNlove great to hear you are presenting on Koop! We will keep working on improving it and welcome your help :)