eldang / elevation_lookups

Takes an input file of paths described as series of points, outputs a file of data about the elevation changes along those paths.
Apache License 2.0
3 stars 2 forks source link

Docker image can't currently be built #34

Open dabreegster opened 10 months ago

dabreegster commented 10 months ago

While running apt-get update:

#10 5.059 W: GPG error: https://apache.jfrog.io/artifactory/arrow/ubuntu jammy InRelease: The following signatures coul
dn't be verified because the public key is not available: NO_PUBKEY 9CBA4EF977CA20B8
#10 5.059 E: The repository 'https://apache.jfrog.io/artifactory/arrow/ubuntu jammy InRelease' is not signed.

This looks like a problem with the base image osgeo/gdal.

@XioNoX, this is blocking me from incorporating #32 into A/B Street. If you have Docker set up, you can reproduce this by building the image locally: docker build -t elevation_lookups . If you have any time to investigate or find an older version of the osgeo/gdal image that works, it'd help. I will get to it when I can but no guarantees :\

dabreegster commented 10 months ago

I tried switching to a newer https://github.com/OSGeo/gdal/pkgs/container/gdal/111206175?tag=ubuntu-full-latest, but then the rasterio wheel fails to build

XioNoX commented 8 months ago

I was able to reproduce the issue last month, but now everything seems to be running fine.

The only (unrelated) change I had to do was to "unpin" rasterio's version from requirements.txt

dabreegster commented 7 months ago

Ditto! I can now build again, but I also had to delete the rasterio line. I'd like to pin everything to specific versions so that the exact same result could be built 5 years from now, and I kind of thought Docker would let us do something like that. But since it's pulling from images that change occasionally, that's hard to achieve. Not a problem right now though.

dabreegster commented 7 months ago

... although the result doesn't run:

Traceback (most recent call last):^M
  File "/elevation/main.py", line 12, in <module>^M
    from data import DataSource^M
  File "/elevation/data.py", line 17, in <module>^M
    import fiona  # type: ignore  # noqa: F401^M
  File "/usr/local/lib/python3.10/dist-packages/fiona/__init__.py", line 86, in <module>^M
    from fiona.collection import BytesCollection, Collection^M
  File "/usr/local/lib/python3.10/dist-packages/fiona/collection.py", line 11, in <module>^M
    from fiona.ogrext import Iterator, ItemsIterator, KeysIterator^M
ImportError: /usr/local/lib/python3.10/dist-packages/fiona/ogrext.cpython-310-x86_64-linux-gnu.so: undefined symbol: _PyGen_Send

I'd like to find pure-Rust options that don't depend on GDAL, or depend on it minimally, to make cross-platform portability and maintenance easier. Not sure when I'll have time to prioritize that, though

dabreegster commented 7 months ago

Another option is to stop rebuilding the Docker image when the input config changes. Providing that file as input instead of baking it in would be simpler

XioNoX commented 7 months ago

Thanks for looking into it !

... although the result doesn't run: How can I try to reproduce it ?

Seems like that issue was discussed in https://github.com/Toblerity/Fiona/issues/1043

dabreegster commented 7 months ago

The full repro would be: 1) Patch in your PR with the new datasource 2) Remove the rasterio line, or otherwise get this container to build again. Then build locally with docker build -t elevation_lookups . 3) Over in the abstreet repo, modify importer/src/map_config.rs and add a case at the bottom for elevation that's || name.city == CityName::new("fr", "brest") 4) Modify convert_osm/src/elevation.rs to use the local Docker container -- change abstreet/elevation_lookups to just elevation_lookups, or whatever name you use in step 2 5) Run ./import.sh --raw --map --city=fr/brest > log 2>&1 6) Check the log after and search for "elevation" to find the error. (Errors here don't stop the importer)

eldang commented 7 months ago

Hello! Sorry I've been so absent for a while. I'm just emerging from a project that took up all of my time for a while. I've also learned a lot about Docker since I last actively worked on this repo, so I'm up for spending some time trying to both fix the immediate issue and making it more robust for the future. I think my agenda will be:

  1. Just get the Docker image buildable again
  2. Update dependency versions, if we haven't already done that as part of 1
  3. Switch the Python packaging to pipenv because on other projects I've found that to be better about not introducing dependency issues like this
  4. As @dabreegster suggests, switch it to taking datasources.json as a runtime input so that the image doesn't have to be rebuilt to add a data source.

Of course I'll welcome help with any of that, but I should also have time to work on this myself over the next 2-3 weeks.

dabreegster commented 7 months ago

No worries and thanks again for all the work on this library! That plan sounds reasonable to me.

I've just started exploring pure Rust solutions in https://github.com/dabreegster/elevation to avoid GDAL dependencies entirely. One idea is to only accept inputs already in EPSG:4326 and just run gdalwarp or similar once for inputs like the King County lidar, simplifying the work that the library would need to do. I also might skip the downloading / file management logic, and instead just have a method that returns a URL of an SRTM or NASADEM file to get, since I've found other projects needing more control over managing assets.