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

COG pipeline #534

Merged
merged 30 commits into from
Jun 11, 2024
Merged

COG pipeline #534

merged 30 commits into from
Jun 11, 2024

Conversation

jterry64
Copy link
Member

@jterry64 jterry64 commented Jun 3, 2024

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
  • Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Check the commit's or even all commits' message styles matches our requested structure.
  • Check your code additions will fail neither code linting checks nor unit test.

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the new behavior?

Create COG assets with specified projection and block size. This creates just one big COG file for all the times.

Does this introduce a breaking change?

  • Yes
  • No

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.90%. Comparing base (a10fa4b) to head (f3d864b).
Report is 8 commits behind head on develop.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #534      +/-   ##
===========================================
+ Coverage    81.71%   81.90%   +0.18%     
===========================================
  Files          125      126       +1     
  Lines         5618     5671      +53     
===========================================
+ Hits          4591     4645      +54     
+ Misses        1027     1026       -1     
Flag Coverage Δ
unittests 81.90% <100.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

batch/scripts/cogify.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@danscales danscales left a comment

Choose a reason for hiding this comment

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

Looks good to me! Some minor suggestions.

"default",
description="Name space to use for COG. "
"This will be part of the URI and will "
"allow to create multiple COGs per version,",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: Change "... per version," to "... per version" (get rid of extra comma). Also, maybe change:

"allow to create multiple COGs per version"

to

"allow creation of multiple COGs per version".

@@ -134,6 +137,19 @@ class PixETLJob(Job):
attempt_duration_seconds = int(DEFAULT_JOB_DURATION * 1.5)


class GDALCOGJob(Job):
"""Use for raster transformations using GDAL Python docker in PixETL
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think better described as:

"""Use for creating COG files using GDAL Python docker in PixETL queue"""

@@ -66,6 +66,8 @@ def get_asset_uri(
uri_constructor: Dict[str, str] = {
AssetType.dynamic_vector_tile_cache: f"{TILE_CACHE_URL}/{dataset}/{version}/dynamic/{{z}}/{{x}}/{{y}}.pbf",
AssetType.static_vector_tile_cache: f"{TILE_CACHE_URL}/{dataset}/{version}/{implementation}/{{z}}/{{x}}/{{y}}.pbf",
# AssetType.cog: f"{TILE_CACHE_URL}/{dataset}/{version}/{implementation}/{{z}}/{{x}}/{{y}}.png",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove this commented out path, unless you think it might be used later?

SRID="$2"
shift # past argument
shift # past value
;;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this new --srid ever used? I don't see it used in cogify.sh

Copy link
Member Author

Choose a reason for hiding this comment

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

We ended up removing it, good catch!

@jterry64 jterry64 merged commit b51894a into develop Jun 11, 2024
2 checks passed
@jterry64 jterry64 deleted the feature/cogify_script branch June 11, 2024 19:48
@manukala6 manukala6 mentioned this pull request Jul 1, 2024
13 tasks
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.

4 participants