feature/53/rename-tree-sources-staging #54
Conversation
Renamed tree_sources_staging to treedata_staging to more easily determine what table this is staging. Other columns remain the same as it reflects the tree-sources file. Two files were added. One is the migrations file, the other is the file created by running the `fetch-tables` script.
tables/public.treedata_staging.sql
Outdated
| geom public.geometry(Point,4326), | ||
| id character varying(255), | ||
| ref character varying(255), | ||
| idname character varying(255), |
There was a problem hiding this comment.
Can you make one more change, this idname is id_source_name now to be more descriptive and make sense if its in some random table.
There was a problem hiding this comment.
The only issue is that ogr2ogr needs an exact name match, so if we did change idname to be id_source_name, we would have to update it in the normalize step. If we want to go that route, I'm happy to also make that change.
I will also comment the top of the migrations file mentioning that the columns must match the normalized geojson perfectly.
There is a rename option in ogr2ogr, but that only works when pulling from shapefiles, not geojson.
Alternatively, we can avoid ogr2ogr and I can write sql statements in the tree-sources repo. That's not a huge lift. If we did that, we could also rename the ref field to something like id_reference.
There was a problem hiding this comment.
It looks like tree-sources already has that change everywhere except on your branch, though the id is camelized idSourceName in the code so you'll have to grab the snake case utility from wtt_server
https://github.com/waterthetrees/wtt_server/blob/main/server/routes/shared-routes-utils.js
There was a problem hiding this comment.
weshouldjustremovetheunderscores... is idsourcename reallysounreadable?
we should just remove the underscores... is idsourcename really so unreadable?
There was a problem hiding this comment.
I think it's fine without the underscores. It's just a staging table. No one should actively need to query this table. Thoughts?
There was a problem hiding this comment.
I'd want to change it everywhere in the code base if we do that. We are using it everywhere and it'd be confusing to have it be different, long term. I am game to have no snake case but will have to make a ton of changes. You up for reviewing PRs in the next few days? I should have time later today to make all the changes required in wtt_server, wtt_front, tree-sources.
There was a problem hiding this comment.
I chatted you in slack, but we should keep a record of it on github as well.
I think it may be easier to keep snake case in the database. ogr2ogr just does a postgres copy command. I can replicate that same thing in node and change the columns. That will be much less work and less risk than changing all three repos.
Thoughts? I can try it out tonight and let you know how it goes.
There was a problem hiding this comment.
After giving it some more thought, for now, can you just change it to id_source_name? It seems like a way bigger change to remove all the snake case/camelCase stuff across all the repos. It is a lot of extra functions and work to convert between snake and camel so it would be nice to get rid of that at some point. Currently however, that is the standard on our code base and I don't want that to block this and all the new functionality going into production.
There was a problem hiding this comment.
If you want to move away from ogr2ogr, let's iterate on this version in another PR so we can get your current work into production as soon as possible. @tzinckgraf Thoughts?
There was a problem hiding this comment.
@zoobot it seems like we are both in agreement on the snake / camel case as per my previous comment.
The move away from ogr2ogr will be in another PR.
This PR is updated.
- update the idname to id_source_name and ref to id_reference
| CREATE UNLOGGED TABLE public.treedata_staging ( | ||
| id_treedata_staging bigint NOT NULL, | ||
| geom public.geometry(Point,4326), | ||
| id character varying(255), |
There was a problem hiding this comment.
This ID is a product of ogr2ogr. We can leave it for now, but we might be able to remove it if we migrate from ogr2ogr.
There was a problem hiding this comment.
That id should be the unique tree id created from https://github.com/waterthetrees/tree-id... Here:
https://github.com/waterthetrees/tree-sources/blob/main/src/stages/normalize.js#L95
It should be the only id actually called "id" in our repos. Is ogr2ogr writing over that unique tree id with a different id?
I just noticed an instance of idName in normalize that needs to be changed to idSourceName. Can submit a PR for that fix after your changes go in.
|
So the unique tree id should be called "id", the unique source id should be called id_source_name/idSourceName. The database serialized table names should be named after the table like id_treedata or id_source or id_crosswalk or id_likes. |
Renamed tree_sources_staging to treedata_staging to more easily determine what table this is staging.
Other columns remain the same as it reflects the tree-sources file.
Two files were added. One is the migrations file, the other is the file created by running the
fetch-tablesscript.This relates to #53