mapbox / mapbox-sdk-js

A JavaScript client to Mapbox services, supporting Node, browsers, and React Native
Other
718 stars 186 forks source link

Deleting icons don't work right away #388

Closed jordymeow closed 4 years ago

jordymeow commented 4 years ago

Hi guys,

It's difficult to find people using this on the web (StackOverflow) and the API behaves weirdly. Basically, I am trying to remove all the icons for a given style, and I upload my own instead. Nothing very complex.

getStyleSprite retrieves all the icons. I am looping through them, and I am calling deleteStyleIcon on every each of them. It's running, without any error.

If I start it again right away, I get the same number of icons. Looping through them and trying to delete them again actually give me errors because they are actually already deleted (Icon not found).

One hour later, indeed, the number if icons is lower, but it's still far from 0. Why is that? Your service actually don't remove them right away, and remove one every 10 minutes or so? That doesn't sound very convenient :)

Another issue is this "Too Many Requests" error sometimes happening. How can we avoid this? Obviously I need to run 80-100 requests, that doesn't sound a lot.

I have been looking for more information/documentation but there is no explanation about all those limitations. Thanks a lot for your help.

andrewharvey commented 4 years ago

See https://docs.mapbox.com/api/maps/#styles-api-restrictions-and-limits for details about rate limits ("Too Many Requests"), you'll just need to space out your requests more and if you hit the rate limit then back off and try again.

You're probably hitting cached responses, so the Mapbox API is still serving the cached object even though it has been deleted.

This repo is for the JS SDK wrapper only, if your issues are coming from the upstream Mapbox API, best reach out to Mapbox Support.

jordymeow commented 4 years ago

Thanks for your reply @andrewharvey.

In the documentation, it's written that: The default rate limit for the Mapbox Styles API endpoint is 2,000 requests per minute. I am doing much fewer request than this, around 30-40 requests per minute. So indeed, that's a bit weird, or maybe the MapBox SDK JS is running extra requests every time? But it doesn't look like so.

I did contact them, but didn't get any reply yet. I thought maybe I could find people running into the same issues here, since those functions are accessible through the MapBox SDK JS.

andrewharvey commented 4 years ago

Yeah I've run into the rate limits quite often too, even when I thought I was under the limits (I use the npm module bottleneck to try and pad out my requests, but it doesn't stop hitting Too Many Requests). It's not ideal but basically I just wait a few seconds and try again.

andrewharvey commented 4 years ago

The other thing I'd mention is try with fresh=true and see if it makes a difference. This isn't documented at https://docs.mapbox.com/api/maps/ however was recently added to this SDK for getStyle. However just now debugging an issue I noticed that other endpoints like listStyles are also serving cached responses and while a cachebuster argument doesn't help, fresh=true did.

So I'd like to add more support for the fresh argument, but it would be good to get some feedback from Mapbox on if this is a supported/document parameter which we can rely on and extend to more api methods.

jordymeow commented 4 years ago

Hi @andrewharvey,

About the limits, in my case, it only concernes the deletions. For all the other actions, it seems the official limits are respected. Odd, but yes, I can live around it :) I am using p-queue, I guess it's similar as bottleneck.

I tried fresh=true but it didn't make a difference. I also try to play with the draft attribute.

I found a way to do what I needed though. Basically, right after I did all my updates on the my main style (which is only used as a reference now), I get the style object, override the sprite URL (which seems to be always wrong, or to reset to something else with default MapBox values) and create a new style from it. MapBox copies the sprite automatically for it. No problem with cache neither since it's a new style, and I can test it right away.

If you are curious, this is the code I used:

const res = await stylesService.getStyle({ ownerId, styleId }).send();
const style = res.body;
let today = dayjs(new Date()).format('YYYY/MM/DD');
newRes = await stylesService.createStyle({ ownerId, style: {
  ...style, 
  sprite: 'mapbox://sprites/myname/' + styleId,
  name: `${style.name} (As of ${today})`
}}).send();
console.log(`New StyleID: ${newStyle.id}`);

Feel free to close my issue. That said, the main issue still exist. I much prefer to do it the way I am doing it now, but we should be able to add icons without any issue. I don't think it's only a cache issue, but I think it's the MapBox API issue (or lack of documentation). Let's see :)

andrewharvey commented 4 years ago

From what I can see it's not an SDK issue, but the upstream API, so for that reason I'll close this ticket.