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

Add clip_geom, buffer and extent under auto_publish conf #887

Merged
merged 6 commits into from
Sep 21, 2023

Conversation

sharkAndshark
Copy link
Collaborator

Partly fix #872

@sharkAndshark sharkAndshark marked this pull request as draft September 18, 2023 06:06
martin/src/pg/configurator.rs Show resolved Hide resolved
pub async fn query_available_tables(pool: &PgPool) -> Result<SqlTableInfoMapMapMap> {
pub async fn query_available_tables(
pool: &PgPool,
clip_geom: Option<bool>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, but it might be better to actually change buffer and clip_geom here to None to indicate that we don't have them yet, and keep them unset until much later as part of the instantiate_tables somewhere around db_inf.srid = srid; line.

Copy link
Collaborator Author

@sharkAndshark sharkAndshark Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyurik I was thinking of this also,how about here:

...
db_inf.srid = srid;
update_id_column(&id2, &mut db_inf, auto_tables);
impose_clip_geom(&mut db_inf, auto_tables);
impose_buffer(&mut db_inf, auto_tables);
...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, just maybe it will make sense to rename the update_id_column to update_auto_fields and do all these steps inside one function

Copy link
Collaborator Author

@sharkAndshark sharkAndshark Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nyurik Have a good day! :)
It's still a draft, but could you help me a code review in case of any big misunderstanding?

@sharkAndshark sharkAndshark changed the title Add clip_geom, buffer under auto_publish conf [WIP] Add clip_geom, buffer under auto_publish conf Sep 19, 2023
@nyurik
Copy link
Member

nyurik commented Sep 21, 2023

I made a few fixes in sharkAndshark#52

Once merged, should be good to go

nyurik and others added 2 commits September 21, 2023 00:21
@nyurik
Copy link
Member

nyurik commented Sep 21, 2023

Looks good, needs docs too

@sharkAndshark sharkAndshark changed the title [WIP] Add clip_geom, buffer under auto_publish conf Add clip_geom, buffer under auto_publish conf Sep 21, 2023
@sharkAndshark sharkAndshark marked this pull request as ready for review September 21, 2023 05:21
@sharkAndshark sharkAndshark changed the title Add clip_geom, buffer under auto_publish conf Add clip_geom, buffer and extent under auto_publish conf Sep 21, 2023
Copy link
Member

@nyurik nyurik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@nyurik nyurik merged commit b2b3e2c into maplibre:main Sep 21, 2023
16 checks passed
@sharkAndshark sharkAndshark deleted the configadjusting branch September 21, 2023 14:19
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.

How to encode all columns as tile properties using config?
2 participants