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

mrc-6064 serve tile data #2

Merged
merged 18 commits into from
Dec 19, 2024
Merged

mrc-6064 serve tile data #2

merged 18 commits into from
Dec 19, 2024

Conversation

EmmaLRussell
Copy link
Contributor

@EmmaLRussell EmmaLRussell commented Dec 2, 2024

This branch adds a database class and controller to return tile data from the server.

It also includes a discoverTileDatasets method which runs on server start-up, which looks for available tile datasets in the data folder. Tile datasets are expected to be provided in folders under data, where the folder name is interpreted as the dataset name. Each folder is expected to contain .mbtiles database files, where the name of each file is interpreted as the level it represents.

Tile data is then returned from endpoint: /tile/:dataset/:level/:z/:x/:y. Discovered datasets are included in app.locals so they can be used by the controller.

Example databases are included - these are levels 0 (country) and 1 (region) that we used for arbomap, in a dataset called "gadm41".

A test page, test.html is also included to sanity check that we're returning valid tile data. Make sure the server is running locally (npm run dev) before opening the page in your browser.

I've included a very basic integration test, and will add a browser test for this test page in a subsequent ticket.

When running the server in a docker container, the required dataset folders should be mounted as a volume to /data.

Comment on lines +12 to +15
tileData = await db.getTileData(
parseInt(z),
parseInt(x),
parseInt(y)
Copy link
Contributor Author

@EmmaLRussell EmmaLRussell Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling for bad requests where these values aren't parseable to be done in https://mrc-ide.myjetbrains.com/youtrack/issue/mrc-6085/Error-handling

Also where requested dataset or level don't exist.

@EmmaLRussell EmmaLRussell marked this pull request as ready for review December 9, 2024 09:57
@EmmaLRussell EmmaLRussell changed the base branch from mrc-6062 to main December 9, 2024 16:47
src/server.ts Outdated Show resolved Hide resolved
src/controllers/tileController.ts Show resolved Hide resolved
src/db/tileDatabase.ts Outdated Show resolved Hide resolved
src/types/db.ts Outdated Show resolved Hide resolved
src/db/tileDatabase.ts Outdated Show resolved Hide resolved
src/db/tileDatabase.ts Show resolved Hide resolved
src/discover.ts Outdated Show resolved Hide resolved
src/discover.ts Outdated Show resolved Hide resolved
src/discover.ts Show resolved Hide resolved
@david-mears-2
Copy link
Contributor

The test.html page appeared to work but gave these console messages, is this expected? Could be firefox-specific. There was nothing odd on the server logs except that there was a constant, fair mix of 200s and 404s (is that expected?)

open test.html
➜  Gtk-Message: 11:29:33.841: Not loading module "atk-bridge": The functionality is provided by GTK natively. Please try to not load it.
[14590, Main Thread] WARNING: GTK+ module /snap/firefox/5361/gnome-platform/usr/lib/gtk-2.0/modules/libcanberra-gtk-module.so cannot be loaded.
GTK+ 2.x symbols detected. Using GTK+ 2.x and GTK+ 3 in the same process is not supported.: 'glib warning', file /build/firefox/parts/firefox/build/toolkit/xre/nsSigHandlers.cpp:201

(firefox:14590): Gtk-WARNING **: 11:29:33.940: GTK+ module /snap/firefox/5361/gnome-platform/usr/lib/gtk-2.0/modules/libcanberra-gtk-module.so cannot be loaded.
GTK+ 2.x symbols detected. Using GTK+ 2.x and GTK+ 3 in the same process is not supported.
Gtk-Message: 11:29:33.940: Failed to load module "canberra-gtk-module"
[14590, Main Thread] WARNING: GTK+ module /snap/firefox/5361/gnome-platform/usr/lib/gtk-2.0/modules/libcanberra-gtk-module.so cannot be loaded.
GTK+ 2.x symbols detected. Using GTK+ 2.x and GTK+ 3 in the same process is not supported.: 'glib warning', file /build/firefox/parts/firefox/build/toolkit/xre/nsSigHandlers.cpp:201

(firefox:14590): Gtk-WARNING **: 11:29:33.942: GTK+ module /snap/firefox/5361/gnome-platform/usr/lib/gtk-2.0/modules/libcanberra-gtk-module.so cannot be loaded.
GTK+ 2.x symbols detected. Using GTK+ 2.x and GTK+ 3 in the same process is not supported.
Gtk-Message: 11:29:33.942: Failed to load module "canberra-gtk-module"

Example of the end of the server logs with 200s and 404s

ffff:127.0.0.1  GET /tile/gadm41/admin0/3/5/5 200  - 2.047 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/5/2 404  - 1.179 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/1/4 200  - 1.534 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/6/4 200  - 1.869 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/1/3 200  - 1.918 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/6/3 200  - 1.512 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/1/5 404  - 1.278 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/6/5 200  - 0.888 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/1/2 404  - 0.682 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/6/2 404  - 0.905 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/0/4 404  - 0.877 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/7/4 200  - 0.862 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/0/3 200  - 0.922 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/7/3 200  - 0.615 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/0/5 404  - 0.783 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/7/5 200  - 0.882 ms
::ffff:127.0.0.1  GET /tile/gadm41/admin0/3/7/2 200  - 0.837 ms

@EmmaLRussell
Copy link
Contributor Author

EmmaLRussell commented Dec 16, 2024

There was nothing odd on the server logs except that there was a constant, fair mix of 200s and 404s (is that expected?)

Yes, that's expected. It's because we stole the tile geojson from the original arbomap dataset and that only covers countries in the equatorial band, but leaflet is trying to load the whole world, so there are some missing tiles returning 404s.

open test.html
➜  Gtk-Message: 11:29:33.841: Not loading module "atk-bridge": The functionality is provided by GTK natively. Please try to not load it.
[14590, Main Thread] WARNING: GTK+ module /snap/firefox/5361/gnome-platform/usr/lib/gtk-2.0/modules/libcanberra-
..etc

I'm not seeing this myself on Firefox. Seems like it's related to accessibility toolkit on Linux, but not sure why this page would be triggering these messages if others don't. https://stackoverflow.com/questions/75406844/not-loading-module-atk-bridge-the-functionality-is-provided-by-gtk-natively

@david-mears-2
Copy link
Contributor

I'm not seeing this myself on Firefox. Seems like it's related to accessibility toolkit on Linux, but not sure why this page would be triggering these messages if others don't. https://stackoverflow.com/questions/75406844/not-loading-module-atk-bridge-the-functionality-is-provided-by-gtk-natively

I tried loading a nearly-empty HTML template and it had the same messages. Maybe it's something particular to my computer then.

@@ -24,6 +24,7 @@
"cors": "^2.8.5",
"express": "^4.21.1",
"morgan": "^1.10.0",
"sqlite": "^5.1.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The slightly nicer sqlite package, which calls itself node-sqlite and is listed as sqlite on npm. We still need sqlite3 which is used as the db driver.

Copy link
Contributor

@david-mears-2 david-mears-2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I still have a single quibble about using the word 'row'...

src/types/db.ts Outdated Show resolved Hide resolved
clarify comment

Co-authored-by: David Mears <60350599+david-mears-2@users.noreply.github.com>
@EmmaLRussell
Copy link
Contributor Author

I've talked Anmol through this now, and he's happy with it, so will merge.

@EmmaLRussell EmmaLRussell merged commit d787157 into main Dec 19, 2024
2 checks passed
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.

2 participants