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

host maps on github #3192

Merged
merged 12 commits into from
Apr 28, 2022
Merged

host maps on github #3192

merged 12 commits into from
Apr 28, 2022

Conversation

jvonau
Copy link
Contributor

@jvonau jvonau commented Apr 26, 2022

Related iiab/iiab-admin-console#483 (comment) "I propose that we put the map-catalog.json on githubusercontent"
Needs cities1000.sqlite and map-catalog.json added to maps and regions.json corrected as per https://github.com/jvonau/maps/pull/1/files
Would need the last commit rolled back before committing to master, it's provided for testing convenience of the new github location scheme until the above changes are implemented in maps.
Tested on 22.04 VM

@jvonau
Copy link
Contributor Author

jvonau commented Apr 26, 2022

iiab/maps#54

@tim-moody
Copy link
Contributor

I thought that our standard was to use the vars in a role for testing, but to make the var in vars/default_vars.yml the definitive value. Is the intent that this is not a user configurable value? I assume it will not be visible to the Admin Console install.

@holta
Copy link
Member

holta commented Apr 26, 2022

I thought that our standard was to use the vars in a role for testing, but to make the var in vars/default_vars.yml the definitive value. Is the intent that this is not a user configurable value? I assume it will not be visible to the Admin Console install.

Sounds good to me!

@jvonau
Copy link
Contributor Author

jvonau commented Apr 26, 2022

I thought that our standard was to use the vars in a role for testing, but to make the var in vars/default_vars.yml the definitive value.

That has not changed, but #3165 didn't take care of default_vars, used defaults/main.yml to point to the testing repo that is still used today. Thought that value would of changed back to iiab/maps once iiab/maps#45 came alive but I'm not the one who approves PRs

Is the intent that this is not a user configurable value? I assume it will not be visible to the Admin Console install.

Any entries in defaults/main.yml or default_vars.yml are still user configurable via local_vars.yml but may not have entries present to do so relying on the default value that is defined.

@holta
Copy link
Member

holta commented Apr 26, 2022

Either way.

Certainly as @tim-moody suggests, if values are changed very rarely every few years, they should likely be in vars/default_vars.yml primarily.

@jvonau
Copy link
Contributor Author

jvonau commented Apr 26, 2022

Guess to sum this up 'iiab_map_url' is going away, 'osm_repo_url' and 'maps_branch' are taking its place becoming the authoritative source pointer once iiab/maps have the required files added. Now 'map_catalog_url' is not quite accurate it's more 'map_mbtiles_url', I can change that while the hood is up or defer that to Adam. Just need to add one more commit to enable default_vars.yml over defaults/main.yml or I can roll back the last commit for the same effect before merging.

@holta
Copy link
Member

holta commented Apr 26, 2022

Either is fine, Thanks Jerry!

@tim-moody
Copy link
Contributor

should the names osm_repo and map_branch sync up, like osm_repo_branch
or even just allow for the branch as part of the url

@georgejhunt
Copy link
Contributor

My strategy is to test the values in osm-vector-maps/defaults/main.yml, which are used for testing massive changes to the map system. And then change the variable name in the roles/install.yml to iiab_map_url (which is set in iiab/vars/default_vars.yml) as a separate, and later PR.

So I'm not in favor of iiab_map_url going away.

@tim-moody
Copy link
Contributor

@georgejhunt I think it's your call.

@jvonau
Copy link
Contributor Author

jvonau commented Apr 26, 2022

iiab_map_url is currently used to retrieve 2 files from http://download.iiab.io where do you propose these files be served from?

@jvonau
Copy link
Contributor Author

jvonau commented Apr 26, 2022

should the names osm_repo and map_branch sync up, like osm_repo_branch or even just allow for the branch as part of the url

That discussion should of been part of PR 3165, I just went with what is already presently defined.

@jvonau
Copy link
Contributor Author

jvonau commented Apr 26, 2022

#3195

@holta
Copy link
Member

holta commented Apr 26, 2022

@georgejhunt I think it's your call.

Thanks @georgejhunt if you can make a recommendation — so @jvonau can refine this PR per your recommendation?

@jvonau
Copy link
Contributor Author

jvonau commented Apr 26, 2022

iiab/maps#55 (comment)

@georgejhunt
Copy link
Contributor

georgejhunt commented Apr 26, 2022 via email

@holta
Copy link
Member

holta commented Apr 26, 2022

good time to let Jerry and Adam take over the development of OSM

@georgejhunt you made all this map/satellite work happen (none of which could have happened without you!) and I apologize I've been quite busy this week with 3 unrelated hardware failures.

But yes we will help package this up as best we can in coming days — calling you when we get stuck.

Thanks to @jvonau's initiative overnight and @tim-moody who's helping a ton here — truly a joint effort and tremendous joint accomplishment with the potential for Many Thousands of kids + teachers to use this over coming years!

@holta
Copy link
Member

holta commented Apr 27, 2022

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.

None yet

4 participants