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

fix(tile-converter): CesiumION tileset URL #2560

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

belom88
Copy link
Collaborator

@belom88 belom88 commented Jul 19, 2023

No description provided.

@belom88 belom88 added this to the v3.4.8 milestone Jul 19, 2023
@belom88 belom88 requested a review from ibgreen July 19, 2023 11:00
Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

I guess it is hard to write a test for this case.

@@ -240,10 +240,14 @@ export default class I3SConverter {

try {
const preloadOptions = await this._fetchPreloadOptions();
let tilesetUrl = inputUrl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The code is a bit difficult to follow. I think it is partly because the existing helper function is called _fetchPreloadOptions but this update makes it clear that the main thing it returns a URL, and beyond that only the headers of the options are used.

_preloadURLAndHeaders() ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The response of prefetch:
image
There are other properties that we don't need

@belom88 belom88 merged commit 4400d4b into master Jul 19, 2023
@belom88 belom88 deleted the vb/fix-tile-converter-cesium-tileset-url branch July 19, 2023 13:30
@dsavinov-actionengine dsavinov-actionengine added the ActionEngine The issue is on ActionEngine controll label Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ActionEngine The issue is on ActionEngine controll
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants