-
Notifications
You must be signed in to change notification settings - Fork 206
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
Modularization of export code #341
Conversation
86a5f92
to
2fdb2d4
Compare
@icarito @sashadev-sky take a look! |
This is looking really good but there are a lot of warnings in the log, and a couple of the steps seem to have a "no directory found" error. But it can start to be pulled into a separate utility now! @icarito should we make a gem out of /lib/exporter.rb, called |
Great! About the Gem, we ourselves don't have an immediate need as the container will likely continue to pull from git directly. |
I guess i'm thinking of breaking this out as a separate Ruby library and independent repository so you don't have to include the whole MapKnitter container to use it, but we can start by just using this container, i guess. Can you try setting this up in a Google Cloud container so we can start trying to use it? Thanks! |
Looking to resolve a few issues before merging:
https://travis-ci.org/publiclab/mapknitter/builds/494340774#L2473
https://travis-ci.org/publiclab/mapknitter/builds/494340774#L2542 I'm going to look for the 900913 error first as it might have cascaded to cause the others. |
So, i deleted the geotiff since we generate it twice in the tests. Then i moved the 'delete directories' test to the end I changed Finally, i added |
Hmm,
ah i see... hang on |
Okay, I see. I'm building a container with gdal 2.4.0 from the Debian repositories for the Testing distribution |
Okay I've built a container including GDAL, Imagemagick and Ruby, and pushed it to the Docker Hub. @jywarren this should be ready for integrating, check out with Regards, |
Thanks Sebastian - linking there now: #349 and I left some thoughts there too. Awesome! |
I believe everything but |
Great, only a couple small things left:
|
|
I think this may have to do with the GDAL version; this is a post by me: |
I'm running GDAL 1.11.3, released 2015/09/16, on my local. Not sure what's in Travis though! |
def self.generate_tiles(key, slug, root) | ||
key = "AIzaSyAOLUQngEmJv0_zcG1xkGq-CXIPpLQY8iQ" if key == "" # ugh, let's clean this up! | ||
key = key || "AIzaSyAOLUQngEmJv0_zcG1xkGq-CXIPpLQY8iQ" | ||
gdal2tiles = 'gdal2tiles.py -k --s_srs EPSG:3857 -t "'+slug+'" -g "'+key+'" '+root+'/public/warps/'+slug+'/'+slug+'-geo.tif '+root+'/public/tms/'+slug+"/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @icarito if you have a moment, i'm a bit stuck here; i think we're down to the GDAL version on this last command. I've been trying to run it locally.
Here are the files that this should be runnable on: https://publiclab.org/i/29584.zip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gdal2tiles.py -k --s_srs EPSG:3857 -t "saugus-landfill-incinerator" -g "AIzaSyAOLUQngEmJv0_zcG1xkGq-CXIPpLQY8iQ" /app/public/warps/saugus-landfill-incinerator/saugus-landfill-incinerator-geo.tif /app/public/tms/saugus-landfill-incinerator/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And Mapknitter.org uses:
$ gdal2tiles.py --version
GDAL 2.1.2, released 2016/10/24
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Travis was same: GDAL 2.1.2, released 2016/10/24
@icarito would you mind trying to resolve the conflicts here? |
* Try running Buster * Include binary GDal * Reorder Dockerfile * Install npm * Tweak apt command * Update Dockerfile * Simplfy * Rollback ruby upgrade and rather install 2.4.4 from rvm * Add login shell to support rvm * Mysql deps * Include git * Fix entry command * Move start command * Revert "Fix entry command" This reverts commit 1833f41. * Remove entrypoint from Dockerfile * Add -l parameter to bash * More deps * Bump mysql2 from 0.3.21 to 0.5.2 Bumps [mysql2](https://github.com/brianmario/mysql2) from 0.3.21 to 0.5.2. - [Release notes](https://github.com/brianmario/mysql2/releases) - [Changelog](https://github.com/brianmario/mysql2/blob/master/CHANGELOG.md) - [Commits](brianmario/mysql2@0.3.21...0.5.2) Signed-off-by: dependabot[bot] <support@dependabot.com> * activerecord-mysql2-adapter * Return to mysql2 < 4 gem and add some grease * Update exporter_test.rb * Update exporter_test.rb * Update exporter.rb * Update exporter_test.rb
71ec8bc
to
bbf1ec8
Compare
Linking to #341, and continuing here. |
@icarito this is complete! |
I started working on a gem, but it's not all the way there. Still, closer to what we'd need for a completely externalized version of this! https://github.com/publiclab/mapknitter-exporter @icarito |
Awesome I'll look at linking it to the Sinatra app for the API |
@jywarren when I run mapknitter locally with Wanted to make sure I understand this PR before looking at @icarito new export API |
also am I remembering currently that the exports are supposed to export the images with the map background, not just the image overlays? |
Oh do we need to open a PR to solve the missing files errors as a distinct
PR?
And no, the export is just the overlays not the background. Thanks!!!!!
…On Thu, Mar 28, 2019, 9:59 AM Sasha Boginsky ***@***.***> wrote:
also am I remembering currently that the exports are supposed to export
the images with the map background, not just the image overlays?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#341 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ1JhEC98TO2LceMK_ob052z2V4eRks5vbMqOgaJpZM4a6kkB>
.
|
@jywarren hmm I don't know. Locally the tests wont pass if your root directory name has spaces in it, like for ex. on my computer I have the Mapknitter repo in a folder "Public Lab". Then it tries to run the commands 2 times: first on To make it pass locally I just did |
ahhhhh, that makes sense. Or we could refactor this to use relative
paths... what about that?
…On Thu, Mar 28, 2019 at 8:38 PM Sasha Boginsky ***@***.***> wrote:
@jywarren <https://github.com/jywarren> hmm I don't know. Locally the
tests wont pass if your root directory name has spaces in it, like for ex.
on my computer I have the Mapknitter repo in a folder "Public Lab". Then it
tries to run the commands 2 times: first on **/public then on
lab/mapknitter/**. But if the root is https://mapknitter.org then it's
fine.
To make it pass locally I just did root.gsub(" ", "\\ ") where
appropriate. Would it make sense to add a PR with this change?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#341 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ9nahwDiUA0hVWf71fYj3s2U5rMBks5vbWBlgaJpZM4a6kkB>
.
|
@jywarren would relative paths still work if the root is |
Hmm. Which PR didn't have them? I believe we were able to resolve them all by re-creating the directories in the tests at the right moments. Can you share your test changes? I do think relative paths may still work, like |
But i just started making some changes to the extracted code here, that'll have to be refactored back in this repo once it stabilizes: publiclab/mapknitter-exporter#6 I am taking notes on the changes in publiclab/mapknitter-exporter#5 but I think you don't have to worry too much at this point... we're hoping to make all of that run itself externally. |
@jywarren I made changes to the code not the tests. Is it the tests I should have been changing? |
* starting to move things into exporter lib, parameterize * additional mysql2 adapter change, sqlite compatibility * moved almost all of exporter code into ruby lib * Try to update GDAL from sources * Update Dockerfile * Try running Buster (publiclab#186) * Try running Buster * Include binary GDal * Reorder Dockerfile * Install npm * Tweak apt command * Update Dockerfile * Simplfy * Rollback ruby upgrade and rather install 2.4.4 from rvm * Add login shell to support rvm * Mysql deps * Include git * Fix entry command * Move start command * Revert "Fix entry command" This reverts commit 1833f41. * Remove entrypoint from Dockerfile * Add -l parameter to bash * More deps * Bump mysql2 from 0.3.21 to 0.5.2 Bumps [mysql2](https://github.com/brianmario/mysql2) from 0.3.21 to 0.5.2. - [Release notes](https://github.com/brianmario/mysql2/releases) - [Changelog](https://github.com/brianmario/mysql2/blob/master/CHANGELOG.md) - [Commits](brianmario/mysql2@0.3.21...0.5.2) Signed-off-by: dependabot[bot] <support@dependabot.com> * activerecord-mysql2-adapter * Return to mysql2 < 4 gem and add some grease * Update exporter_test.rb * Update exporter_test.rb * Update exporter.rb * Update exporter_test.rb
Re: #298 just starting here!