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

Adding french aerial imagery and french style openstreetmap #194

Closed
wants to merge 5 commits into from

Conversation

Projects
None yet
9 participants
@sidjy
Copy link
Contributor

commented Oct 11, 2016

According to http://openstreetmap.fr/bdortho, french national institute of geographic and forest information (IGN) authorizes openstreetmap to use its aeral imagery

@tyrasd
Copy link
Member

left a comment

Hi! Thanks for the PR. This looks like a very valuable addition to the editor layer index. 👍

I've added a couple of inline remarks that should be addressed (especially the one regarding 403 errors returned by the tile server URL).

Also, can you please run make and commit the changes to the generated output files to the PR as well?

Thanks!

@@ -10,6 +10,7 @@
"url": "http://{switch:a,b,c}.layers.openstreetmap.fr/bano/{zoom}/{x}/{y}.png",
"max_zoom": 20,
"min_zoom": 12,
"overlay": true,

This comment has been minimized.

Copy link
@tyrasd

tyrasd Oct 22, 2016

Member

very minor nitpick: can you change the indention here from tab to spaces?

This comment has been minimized.

Copy link
@tyrasd

tyrasd Oct 22, 2016

Member

(PS: there's two or three more places in other files of this PR where indention uses tabs instead of spaces.)

This comment has been minimized.

Copy link
@sidjy

sidjy Oct 23, 2016

Author Contributor

done (hope I didn't miss any indention)

"required": true
},
"description": "French IGN aeral imagery",
"url": "http://proxy-ign.openstreetmap.fr/bdortho/{zoom}/{x}/{y}.jpg",

This comment has been minimized.

Copy link
@tyrasd

tyrasd Oct 22, 2016

Member

This URL only gives me HTTP/403 errors, for example when trying to access http://proxy-ign.openstreetmap.fr/bdortho/12/2103/1461.jpg – is that just a temporary hickup or has the URL been changed?

This comment has been minimized.

Copy link
@tyrasd

tyrasd Oct 22, 2016

Member

Is the tile server also available via https?

This comment has been minimized.

Copy link
@Robou

Robou Oct 22, 2016

@tyrasd yes it seems that it can be accessed both via http and https.
They are talking about this on this openstreetmapFrance thread.

This comment has been minimized.

Copy link
@tyrasd

tyrasd Oct 22, 2016

Member

I see. There's also a ticket on the iD repository, apparently: openstreetmap/iD#3197

"attribution": {
"url": "http://openstreetmap.org/",
"text": "© OpenStreetMap contributors, CC-BY-SA",
"required": false

This comment has been minimized.

Copy link
@tyrasd

tyrasd Oct 22, 2016

Member

Why is it not required? 😕

This comment has been minimized.

Copy link
@sidjy

sidjy Oct 23, 2016

Author Contributor

because I did like the GermanStyle....but you're right, it should be required, I just changed it

@tyrasd

This comment has been minimized.

Copy link
Member

commented Oct 22, 2016

related ticket: openstreetmap/iD#3420

@tyrasd

This comment has been minimized.

Copy link
Member

commented Oct 22, 2016

Ohh, I see that the 403's above are caused by a server-side user-agent or http-referrer check that basically only allows tiles to be accessed by JOSM or iD. That's a bit unfortunate, since this repository is intended for more than just the iD editor (other OSM editors should be able to use the layers as well).
//cc @cquest

@althio

This comment has been minimized.

Copy link

commented Oct 30, 2016

@tyrasd Maybe not the ideal solution ultimately, but this is the best @cquest can do right now.
(JOSM then iD mappers are waiting for this in France.)

I guess server-side check can be added for other OSM editors, as OpenStreetMap France and IGN want to go for a white-list approach.

@pnorman

This comment has been minimized.

Copy link
Member

commented Oct 30, 2016

👎 if it requires specific user-agents or referers.

@althio

This comment has been minimized.

Copy link

commented Oct 31, 2016

@pnorman

👎 if it requires specific user-agents or referers.

I would like more details here, any explanation or guidelines about this? Is it technical, legal, ...?

To expose a bit more the context and current situation:

Any concerns/suggestions welcome.

@Robou

This comment has been minimized.

Copy link

commented Oct 31, 2016

Hi, Can't we split this Pull Request into two parts/PR ?

  1. one for adding osmFR map rendering : "OpenStreetMap (French Style)"
  2. one for the BDorthoIGN imagery automatically accessible on iD
    Because the first one seems to me easier and does not cause any issue. Sory if this is irrevelant, I am pretty noob and maybe I didn't catch the true meaning of the lines of code in the PR.
@bhousel

This comment has been minimized.

Copy link
Member

commented Oct 31, 2016

Hi, Can't we split this Pull Request into two parts/PR ?

There is no need to do that:

  1. iD won't be showing "French Style" mapnik tiles. You can certainly include them in the index, but we would blacklist that source like we do with other local osm tilesets.
  2. iD can't display imagery from a server that returns 403 based on a useragent/referrer check. Again, you can include it in the index if you think it's useful to some other editor, but we'd just blacklist it on the iD side.

Neither part of this PR will make it into iD.

@pnorman

This comment has been minimized.

Copy link
Member

commented Oct 31, 2016

I would like more details here, any explanation or guidelines about this? Is it technical, legal, ...?

This index is for more than just JOSM and iD.

@althio

This comment has been minimized.

Copy link

commented Nov 3, 2016

@cquest
according to openstreetmap/iD#3420 (comment) or openstreetmap/iD#3197 (comment) one way to go is access_token or key, possibly with public token / public key

general key provided for OSM editors, and therefore requires no registration. However, the accesses with this key are limited (like https://wiki.openstreetmap.org/wiki/WikiProject_Austria/geoimage.at#JOSM )

"url": "http://gis.lebensministerium.at/wmsgw/?key=4d80de696cd562a63ce463a58a61488d&FORMAT=image/jpeg&VERSION=1.1.1&SERVICE=WMS&REQUEST=GetMap&Layers=Luftbild_MR,Luftbild_1m,Luftbild_8m,Satellitenbild_30m&SRS={proj}&WIDTH={width}&HEIGHT={height}&BBOX={bbox}"

@althio

This comment has been minimized.

Copy link

commented Nov 3, 2016

@bhousel

  1. iD won't be showing "French Style" mapnik tiles. You can certainly include them in the index, but we would blacklist that source like we do with other local osm tilesets.

Fair enough, potential access in other editors is ok.

  1. iD can't display imagery from a server that returns 403 based on a useragent/referrer check. Again, you can include it in the index if you think it's useful to some other editor, but we'd just blacklist it on the iD side.

Well iD can, it works with custom link
http(s)://proxy-ign.openstreetmap.fr/bdortho/{z}/{x}/{y}.jpg
So why the blacklist?

@bhousel

This comment has been minimized.

Copy link
Member

commented Nov 3, 2016

Well iD can, it works with custom link
http(s)://proxy-ign.openstreetmap.fr/bdortho/{z}/{x}/{y}.jpg
So why the blacklist?

It does not work:

screenshot 2016-11-03 09 56 31

@althio

This comment has been minimized.

Copy link

commented Nov 3, 2016

Hmmm yes apparently still not right over http but works with https.

@pnorman

This comment has been minimized.

Copy link
Member

commented Nov 3, 2016

Hmmm yes apparently still not right over http but works with https.

I get a 403 over https too

image

@cquest

This comment has been minimized.

Copy link

commented Nov 3, 2016

You'll have access granted when using iD on openstreetmap.org web site.
If we can set a separate instance of iD with this layer in the preset, it may be another way to offer iD editing with this aerial imagery without having to register it in the global list.
Is this something that can be configured in iD code somewhere ? (never looked at it)

@pnorman

This comment has been minimized.

Copy link
Member

commented Nov 4, 2016

You'll have access granted when using iD on openstreetmap.org web site.

Ya, that's going to make it unsuitable for here.

If we can set a separate instance of iD with this layer in the preset, it may be another way to offer iD editing with this aerial imagery without having to register it in the global list.

You'll need to change the layers part of the iD build process which shouldn't be too hard. If you have difficulty you're best off raising an issue on the iD tracker.

@bhousel

This comment has been minimized.

Copy link
Member

commented Nov 4, 2016

If we can set a separate instance of iD with this layer in the preset, it may be another way to offer iD editing with this aerial imagery without having to register it in the global list.
Is this something that can be configured in iD code somewhere ? (never looked at it)

I agree with @pnorman that this source does not belong in the editor-layer-index.

If you want to run your own fork of iD that includes it, you can add the source to a whitelist in update_imagery.js.

At this point, you're talking about forking the OSM tools just to support one imagery source with onerous usage requirements. My strong recommendation is still to work with the imagery provider to actually open up their imagery in a way that lets everyone use it.

@bhousel bhousel closed this Nov 4, 2016

@tyrasd

This comment has been minimized.

Copy link
Member

commented Nov 4, 2016

My strong recommendation is still to work with the imagery provider to actually open up their imagery in a way that lets everyone use it.

👍

PS: @sidjy: feel free to open a new pull request that only adds the French osm tiles.

@althio

This comment has been minimized.

Copy link

commented Nov 7, 2016

@bhousel

I agree with @pnorman that this source does not belong in the editor-layer-index.

Well, we have a source, not fully open, but granting the right to use for OSM editing purposes.
It seemed natural to add it in the common repo for OSM editors.

I understand that you all ( @tyrasd @bhousel @pnorman ) oppose the current implementation (which tries to provide extra steps to ensure it is only used for OSM editors, but this is not done quite right (yet?)).
However I don't understand why you think the source itself does not belong here.

My strong recommendation is still to work with the imagery provider to actually open up their imagery in a way that lets everyone use it.

It took some years and effort from several OSM members in the French community to get there with the national mapping agency. It is a significant first step for them and for us. Not an argument, just a comment: I dare say it will not give a very good signal to open up their data if we make little use of the current agreement.

Please be sure that our final goal is free geo data and promoting actual open data. Only, with external partners, sometimes we just have to learn to walk before we can run.

@cquest

If we can set a separate instance of iD with this layer in the preset, it may be another way [...]

@bhousel ;

If you want to run your own fork of iD that includes it, you can add [...]
At this point, you're talking about forking the OSM tools just to support one imagery source with onerous usage requirements.

I cannot speak for @cquest, but I think he is just trying to find proposals/solutions, and the easiest if possible.
If every other way is blocked, he is standing and looking at the forking route. Not sure he will walk the path, just an option to look at. Anyway, thanks for the pointers.

To reiterate: currently iD and JOSM users already have access, only with custom URL. We are just trying to make it easier and transparent for beginner contributors. We would also like the imagery to be available for other OSM editors.

@bhousel

This comment has been minimized.

Copy link
Member

commented Nov 7, 2016

I understand that you all ( @tyrasd @bhousel @pnorman ) oppose the current implementation (which tries to provide extra steps to ensure it is only used for OSM editors, but this is not done quite right (yet?)).
However I don't understand why you think the source itself does not belong here.

None of us have been able to get the source to work in testing. I think (speaking for myself) if proxy-ign returned images, I would have no issue with including it in the index.

Even if the proxy returned images that all said "visit http://whatever.fr to accept the terms", and then the user does that step and the proxy starts serving images, I'd be ok with that too.

@Dominik-K

This comment has been minimized.

Copy link

commented Apr 6, 2017

@bhousel Did you also try the TLS connection? Your screenshot just shows the HTTP test. In the OSM France blog, they explicitly mention the https channel for iD.

It worked fine for me with iD on the official openstreetmap.org website using https://proxy-ign.openstreetmap.fr/bdortho/{zoom}/{x}/{y}.jpg as a custom background layer.

@pnorman Did you try it on the OSM website, as well?

@pnorman

This comment has been minimized.

Copy link
Member

commented Apr 6, 2017

@pnorman Did you try it on the OSM website, as well?

No - nor do I feel the need to. osm.org is not the only referrer used for people accessing imagery that is in this index.

@bhousel

This comment has been minimized.

Copy link
Member

commented Apr 6, 2017

@bhousel Did you also try the TLS connection? Your screenshot just shows the HTTP test. In the OSM France blog, they explicitly mention the https channel for iD.

At the time, I only tested with the http source string that was submitted in the PR.

It looks like even with https I get 403 Forbidden:

screenshot 2017-04-06 14 26 16

I'm running iD locally though, not on openstreetmap.org, so that's probably why.

@grischard

This comment has been minimized.

Copy link
Collaborator

commented Feb 25, 2018

Please test #418 for a fix that adds a token to use the French IGN imagery in ID.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.