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

MEL - datahub - tile WMS #887

Merged
merged 1 commit into from
May 29, 2024
Merged

MEL - datahub - tile WMS #887

merged 1 commit into from
May 29, 2024

Conversation

rcaplier
Copy link
Collaborator

@rcaplier rcaplier commented May 28, 2024

Description

This PR introduces a new parameter in the default.toml config file: do_not_tile_wms which is false by default.

Screenshots

  • When the parameter is set to false (by default), we can see on this dataset in the network that 15 tiles are fetched :

image

  • When the parameter is set to true, we can see that only one image is fetched :

image

Quality Assurance Checklist

  • Commit history is devoid of any merge commits and readable to facilitate reviews
  • If new logic ⚙️ is introduced: unit tests were added
  • If new user stories 🤏 are introduced: E2E tests were added
  • If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
  • If breaking changes 🪚 are introduced: add the breaking change label
  • If bugs 🐞 are fixed: add the backport <release branch> label
  • The documentation website 📚 has received the love it deserves

This work is sponsored by MEL.

@rcaplier rcaplier self-assigned this May 28, 2024
@rcaplier rcaplier force-pushed the dh-do-not-tile-wms-by-default branch from 7825a11 to 7b105be Compare May 28, 2024 14:50
Copy link
Contributor

github-actions bot commented May 28, 2024

Affected libs: feature-map, feature-dataviz, feature-record, feature-router, util-app-config,
Affected apps: metadata-editor, datahub, demo, webcomponents, search, map-viewer,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@coveralls
Copy link

coveralls commented May 28, 2024

Coverage Status

coverage: 84.492% (+0.2%) from 84.26%
when pulling 7c8f48d on dh-do-not-tile-wms-by-default
into 16f429a on main.

Copy link
Contributor

github-actions bot commented May 28, 2024

📷 Screenshots are here!

@jahow jahow added the breaking change Requires an adaptation by the user to keep feature parity label May 28, 2024
Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Thanks! Haven't tested but it looks good. We have to keep in mind to 1/ document the setting here and 2/ make sure that the next version will mention this change (exp. the gutter being gone) and let people know that they might need to use this setting if they have artefacts close to tile borders.

@rcaplier rcaplier force-pushed the dh-do-not-tile-wms-by-default branch from 7b105be to 5e97027 Compare May 29, 2024 07:49
@rcaplier rcaplier force-pushed the dh-do-not-tile-wms-by-default branch 2 times, most recently from 0bf131b to 749a57d Compare May 29, 2024 09:08
docs/guide/configure.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@tkohr tkohr left a comment

Choose a reason for hiding this comment

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

Thanks @rcaplier, tested it and it works fine! Geowebcache now responds with a Geowebcache-Cache-Result: HIT when keeping the default behavior (WMS being tiled) 🙂

conf/default.toml Outdated Show resolved Hide resolved
@rcaplier rcaplier force-pushed the dh-do-not-tile-wms-by-default branch 2 times, most recently from 6653367 to f0ae0ef Compare May 29, 2024 12:30
@tkohr
Copy link
Collaborator

tkohr commented May 29, 2024

Thanks for the adaptions @rcaplier! I think this one can be merged once the conflict is resolved. Watch out rebasing, there's a new attributions property now.

@rcaplier rcaplier force-pushed the dh-do-not-tile-wms-by-default branch from f0ae0ef to 7c8f48d Compare May 29, 2024 15:39
@rcaplier rcaplier merged commit cc25794 into main May 29, 2024
9 checks passed
@cmoinier cmoinier deleted the dh-do-not-tile-wms-by-default branch May 30, 2024 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Requires an adaptation by the user to keep feature parity minor feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants