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

map package #95

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MockingMagician
Copy link

The purpose of this package is simply to quickly view an address through presearch.
Capture d’écran de 2021-11-21 02-21-17

designing map package
based on openstreetmap and leaflet
@MockingMagician
Copy link
Author

Hi @colinpape, @treygrainger, there is someone for checking pull request ?

@jejopl
Copy link
Collaborator

jejopl commented Dec 15, 2021

Hey @MockingMagician, sorry for the delay, two things:

I see that you are requesting nominatim.openstreetmap.org API. Do you know how many requests are we allowed to do per day? We need to know this information before we will push it to production.

The second thing is that you need to include js and css files into the package itself. We are not allowing scripts from external urls for the security reasons

@treygrainger
Copy link
Member

treygrainger commented Mar 15, 2022

It's also not okay to rendering openstreetmap tiles from an external url directly on the page, as this exposes end user IP addresses to the openstreetmap servers.

I think ideally to pull this off we would either:

  1. Run an openstreetmap tile server on Presearch's servers (and eventually the nodes in a decentralized fashion), or
  2. Load the tiles through the Presearch image cache to hide the end-user IPs

#2 would be the easiest short-term, but #1 would be the most effective ultimate solution. We'd need to create an image cache API to do #2 appropriately (securely and so the image cache is only used for the packages and not as an open proxy), though, which would take a little extra work.

@MockingMagician
Copy link
Author

Hi @treygrainger

I totally agree about not exposing the end user's IP. You should think of this package as a POC for a map on presearch.

Is there any way to help you implement this functionality on the server or nodes on presearch?

CU

@treygrainger
Copy link
Member

Hi @MockingMagician sounds good. FYI, we've just created a new #open-source-dev channel in our Discord group (https://discord.com/invite/VZaDSheRvR) for folks to collaborate on packages. Happy to chat here on github, but if you want you're also free to join there if you want some more interactive discussion.

re: the server implementation, ultimately I think we're going to need a map tile server implementation. Probably the easiest way to do this would be:

  1. Set it up in a centralized fashion (so we can launch the feature)
  2. Figure out how to partition and decentralize the data across the nodes, as well as update on an ongoing basis.

#2 is a bit more tricky and could maybe be a longer term goal. After our mainnet launch, we'll be working on search indexes, which will include partitioning and more sophisticated query routing logic. We could potentially piggy-back on that at the time and think of the map tiles almost like additional partitioned indexes (but just lookups of tiles as opposed to search indexes).

If you're interested in assisting on the server side with the creation of the tile server, we could create a new repository and give you write access and enable you to work on building it out. There are several online tutorials for doing this (docker image would probably be best), though I think the trick would be to try to optimize for the particular use case and zoom level so as not to require insane amounts of memory to run this.

Let us know if you'd be interested in pursuing this. It's something the core dev team will eventually take on if you or someone from the community does not, but it will be a while before we get to it.

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