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

Support overriding maps config from OSD config yml file #202

Merged
merged 2 commits into from
Jan 11, 2023

Conversation

junqiu-lei
Copy link
Member

@junqiu-lei junqiu-lei commented Jan 11, 2023

Signed-off-by: Junqiu Lei [email protected]

Description

Allow user to override maps service url configuration from OpenSearch-Dashboard config yml file by adding:

custom_import_map_dashboards.opensearchVectorTileDataUrl: "https://tiles.maps.opensearch.org/data/v1.json"
custom_import_map_dashboards.opensearchVectorTileStyleUrl: "https://tiles.maps.opensearch.org/styles/basic.json"
custom_import_map_dashboards.opensearchVectorTileGlyphsUrl: "https://tiles.maps.opensearch.org/fonts/{fontstack}/{range}.pbf"

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@junqiu-lei junqiu-lei requested a review from a team January 11, 2023 07:53
@junqiu-lei junqiu-lei self-assigned this Jan 11, 2023
@junqiu-lei junqiu-lei changed the title Get map config from OSD config yml Support overriding maps config from OSD config yml file Jan 11, 2023
Signed-off-by: Junqiu Lei <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2023

Codecov Report

Merging #202 (80a74cb) into main (e5d98f2) will decrease coverage by 0.21%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #202      +/-   ##
==========================================
- Coverage   87.64%   87.42%   -0.22%     
==========================================
  Files           6        6              
  Lines         178      175       -3     
  Branches       24       24              
==========================================
- Hits          156      153       -3     
  Misses         16       16              
  Partials        6        6              
Impacted Files Coverage Δ
...Dashboards/plugins/dashboards-maps/common/index.ts 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@ruanyl ruanyl left a comment

Choose a reason for hiding this comment

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

Approved with a minor comment

public/index.ts Show resolved Hide resolved
@junqiu-lei junqiu-lei merged commit b180af9 into opensearch-project:main Jan 11, 2023
@junqiu-lei junqiu-lei deleted the config_from_yml branch January 11, 2023 15:25
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 11, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 11, 2023
junqiu-lei added a commit that referenced this pull request Jan 11, 2023
junqiu-lei added a commit that referenced this pull request Jan 11, 2023
@@ -23,9 +23,6 @@ export {
PLUGIN_NAME,
};

export const MAP_VECTOR_TILE_BASIC_STYLE = 'https://tiles.maps.opensearch.org/styles/basic.json';
export const MAP_GLYPHS = 'https://tiles.maps.opensearch.org/fonts/{fontstack}/{range}.pbf';
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have to move font stack to config?

Copy link
Member Author

@junqiu-lei junqiu-lei Jan 11, 2023

Choose a reason for hiding this comment

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

It moved to config.ts with the default value so that the url can be overridden configured from OSD config url

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants