Mapnik omnivore refactor #1199

GretaCB · 2015-02-19T20:33:06Z

Prep Studio for integrating the new mapnik-omnivore refactor

Conflicts: lib/source.js package.json

springmeyer · 2015-02-20T21:55:51Z

will this address #1156?

GretaCB · 2015-02-23T19:29:30Z

@springmeyer Yes. Studio no longer directly calls any mapnik-omnivore functions, so this error shouldn't happen anymore. Also, added some error handling for the mapnik call.

springmeyer · 2015-02-23T19:57:32Z

@GretaCB what value does this commit add? f241d39. To me that just catches the Mapnik error and throws another. I'm biased, but personally I'd like to see the Mapnik error reported since it may contain helpful details to know what went wrong.

GretaCB · 2015-02-23T20:17:57Z

Good point. Will pass in the caught error.

On Mon, Feb 23, 2015 at 2:57 PM, Dane Springmeyer [email protected]
wrote:

@GretaCB https://github.com/GretaCB what value does this commit add?
f241d39
f241d39.
To me that just catches the Mapnik error and throws another. I'm biased,
but personally I'd like to see the Mapnik error reported since it may
contain helpful details to know what went wrong.

—
Reply to this email directly or view it on GitHub
#1199 (comment).

GretaCB · 2015-02-27T18:17:43Z

@samanpwbb @springmeyer This is ready to merge. What is the process for deploy/release?

cc @rclark

wilhelmberg · 2015-02-27T19:12:07Z

My 2 cents concerning f241d39

If I decide to catch an error and re-throw (happens rarely, most of the time to do some extra error logging), then I like to include the original error plus some additional information to provide more context and make debugging/error hunting easier, e.g.:

throw new Error(util.format(
    'Could not load datasource "%s" for layer "%s": %s'
    , opts
    , l.id
    , err
));

I'm not familiar with the style rules of this code base and haven't checked if there is a general catch somewhere, but maybe that's what @springmeyer meant, too.

springmeyer · 2015-03-03T01:37:55Z

@samanpwbb @springmeyer This is ready to merge. What is the process for deploy/release?

Because this a fairly big upgrade I think it would be worth testing the published packages next. That looks like:

Publish packages by committing to this branch with `git commit -a -m "[publish mapnik-om-refactor]"

Details on getting the package urls at https://github.com/mapbox/mapbox-studio/blob/mb-pages/CONTRIBUTING.md#packaging.

Then test the OS X package locally (make sure it opens, can load some data), and then test the Windows package - plan over at Developer docs on how to manually test on windows #1218.

@BergWerkGIS yes, exactly - that is what I meant - that at least keeping the original error is desirable, or adding to it like you've demonstrated.

GretaCB · 2015-03-03T03:15:10Z

So sorry @BergWerkGIS , I missed your response. Thanks for the example, I will commit a fix with the original error concatenated.

Awesome plan, thanks @springmeyer !

samanpwbb · 2015-03-20T19:38:57Z

app/source.js

-                var center = metadata.center;
-                var zoom = Math.max(metadata.minzoom, view.model.get('minzoom'));
-                map.setView([center[1], center[0]], zoom);
+                map.fitBounds([


to account for #1270 - fitBounds should disable animation as well.

GretaCB · 2015-03-20T19:42:01Z

Note to self:

disable animation in map.fitBounds...set animation: false (ref https://github.com/mapbox/mapbox-studio/pull/1270/files)

springmeyer · 2015-03-27T04:19:47Z

I'm stealing this as I realize I need this to be able to finish #1287. So, working on bringing this patch up to date now.

springmeyer · 2015-03-27T07:59:44Z

Successfully integrated this into #1287. Thanks @GretaCB!

…to stylesAndSources * 'mb-pages' of https://github.com/mapbox/mapbox-studio: [publish 4abb776] Start changelog - refs #1301 cleanup and update deps sync templates for topojson/geojson with ogr build out test documentation further fix test fixtures debug why publishing is not being triggered [publish node-mapnik-3.2.0] [publish node-mapnik-3.2.0] test against upcoming node-mapnik 3.2.1 use [email protected] update and integrate mapnik-om upgrade from #1199 upgrade mapnik related deps upgrade to [email protected]

…tudio into stylesAndSources * 'stylesAndSources' of https://github.com/mapbox/mapbox-studio: [publish 4abb776] Changes the Projects button (and occurences of 'Project(s)') to 'Styles & Sources' this change is because 'Projects' is a name we've given to things made with the web editor which have no connection to studio Start changelog - refs #1301 cleanup and update deps sync templates for topojson/geojson with ogr build out test documentation further fix test fixtures debug why publishing is not being triggered [publish node-mapnik-3.2.0] [publish node-mapnik-3.2.0] test against upcoming node-mapnik 3.2.1 use [email protected] update and integrate mapnik-om upgrade from #1199 upgrade mapnik related deps upgrade to [email protected]

gretacb added 9 commits February 9, 2015 16:27

remove exported extent function and add error check

2c6c1e7

Merge branch 'mb-pages' into mapnik-om-refactor

ef3e6c5

remove unnecessary omnivore logic

04f28d7

Merge branch 'mb-pages' into mapnik-om-refactor

b636f95

match mapnik versions

cb2cd68

add topojson/geojson data types and dupe test

36f0e32

fix zoomToLayer, fix tests and layer name logic

637df84

Merge branch 'mb-pages' into mapnik-om-refactor

6125185

Conflicts: lib/source.js package.json

remove redundant test

fbc38de

add error handling for mapnik function call

f241d39

reverting try/catch

0597b3f

GretaCB mentioned this pull request Feb 24, 2015

Module-based refactor mapbox/mapnik-omnivore#84

Merged

gretacb added 2 commits February 27, 2015 11:58

Merge branch 'mb-pages' into mapnik-om-refactor

1e11231

add published mapnik-omnivore package

76a5602

springmeyer mentioned this pull request Mar 3, 2015

Developer docs on how to manually test on windows #1218

Open

springmeyer mentioned this pull request Mar 3, 2015

mapnik-omnivore may return error when detecting extent #1156

Closed

Include original error message from mapnik

ad791a4

rclark mentioned this pull request Mar 20, 2015

Delay setView when adding layers to source mode #1269

Closed

samanpwbb reviewed Mar 20, 2015
View reviewed changes

This was referenced Mar 24, 2015

Fix crash when uploading postgis source #966

Closed

[wip] node-mapnik 3.2.0 #1287

Merged

springmeyer mentioned this pull request Mar 27, 2015

disable animation on setView, fix #1254 #1270

Merged

springmeyer pushed a commit that referenced this pull request Mar 27, 2015

update and integrate mapnik-om upgrade from #1199

823ffb9

springmeyer closed this Mar 27, 2015

springmeyer deleted the mapnik-om-refactor branch March 27, 2015 07:59

springmeyer added this to the v0.2.8 milestone Mar 27, 2015

springmeyer pushed a commit that referenced this pull request Apr 2, 2015

update and integrate mapnik-om upgrade from #1199

1ccf37c

This was referenced Jul 6, 2015

Calculate extents of PostGIS-loaded data #731

Closed

Application suddenly quits when uploading source #1401

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mapnik omnivore refactor #1199

Mapnik omnivore refactor #1199

GretaCB commented Feb 19, 2015

springmeyer commented Feb 20, 2015

GretaCB commented Feb 23, 2015

springmeyer commented Feb 23, 2015

GretaCB commented Feb 23, 2015

GretaCB commented Feb 27, 2015

wilhelmberg commented Feb 27, 2015

springmeyer commented Mar 3, 2015

GretaCB commented Mar 3, 2015

samanpwbb Mar 20, 2015

GretaCB commented Mar 20, 2015

springmeyer commented Mar 27, 2015

springmeyer commented Mar 27, 2015

Mapnik omnivore refactor #1199

Mapnik omnivore refactor #1199

Conversation

GretaCB commented Feb 19, 2015

springmeyer commented Feb 20, 2015

GretaCB commented Feb 23, 2015

springmeyer commented Feb 23, 2015

GretaCB commented Feb 23, 2015

GretaCB commented Feb 27, 2015

wilhelmberg commented Feb 27, 2015

springmeyer commented Mar 3, 2015

GretaCB commented Mar 3, 2015

samanpwbb Mar 20, 2015

Choose a reason for hiding this comment

GretaCB commented Mar 20, 2015

Note to self:

springmeyer commented Mar 27, 2015

springmeyer commented Mar 27, 2015