-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
Optimize generating project data #2214
base: development
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
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.
Some nice updates!
I think there could be some logic issues that I commented on, but could be wrong π
Let me know your thoughts
for f in batch | ||
] | ||
|
||
async with db.cursor() as cur: |
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.
This will open a cursor for every feature in the project surely, as it's in the for loop?
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.
The previous approach inserted the featcol all in one go I think
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.
yeah you are right i can change that to use one cursor for all the batches.
@@ -353,43 +311,31 @@ def add_required_geojson_properties( | |||
This step is required prior to flatgeobuf generation, | |||
else the workflows of conversion between the formats will fail. | |||
""" | |||
for feature in geojson.get("features", []): | |||
properties = feature.get("properties", {}) | |||
current_timestamp = datetime.now().strftime("%Y-%m-%dT%H:%M:%S") |
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.
We have a helper function from postgis_utils timestamp()
that returns a current timezone aware timestamp in UTC
What type of PR is this? (check all applicable)
Related Issue
Describe this PR
Screenshots
N/A
Alternative Approaches Considered
Did you attempt any other approaches that are not documented in code?
Review Guide
Notes for the reviewer. How to test this change?
Checklist before requesting a review
[optional] What gif best describes this PR or how it makes you feel?