Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Allow requesting optional resources #5027

Merged
merged 1 commit into from
May 18, 2016
Merged

Allow requesting optional resources #5027

merged 1 commit into from
May 18, 2016

Conversation

kkaefer
Copy link
Member

@kkaefer kkaefer commented May 13, 2016

Right now, when calling FileSource::request(), we're making every effort to obtain the data. However, for #123, we need a more relaxed way to request things that can be obtained with low effort (e.g. by loading it from a cache, but not downloading it from the internet).

@kkaefer
Copy link
Member Author

kkaefer commented May 13, 2016

/cc @jfirebaugh

@jfirebaugh
Copy link
Contributor

Looks good! (For anyone else reviewing, the addition of Resource::necessity was in cd65a43.)

Checking my understanding of how this will be used (c.f. #4844):

  1. Source will make an Optional request for the ideal tile.
  2. If unfulfilled, it will make a Required request for the ideal tile. Concurrently, it will make Optional requests for successive parent tiles until either one of those is fulfilled, or the Required request for the ideal tile is fulfilled.
  3. Otherwise, if fulfilled, it will make a Required request for the ideal tile, passing the prior headers so that the request doesn't go back through the cache pathway.

Is this right? The (minor) gap I see with this strategy is that in step 2, it looks like the Required request for the ideal tile will go back through the cache pathway, even though there's known to be nothing there.

@kkaefer
Copy link
Member Author

kkaefer commented May 13, 2016

For anyone else reviewing, the addition of Resource::necessity was in cd65a43

Yeah, that was an oversight; sorry about that.

Is this right?

Yup, correct. Essentially it'll climb up the tile pyramid to see if something is in the cache without requesting from the internet that is not an ideal tile.

The (minor) gap I see with this strategy is that in step 2, it looks like the Required request for the ideal tile will go back through the cache pathway, even though there's known to be nothing there.

In step 2, it knows that the request is unfulfilled and can set priorExpires to 0, which will make DefaultFileSource skip the cache. I'm wondering if we should make this behavior more explicit, e.g. by changing necessity to something that explicitly tells the FileSource that it should make a cache request xor http request, however, I'm not sure how to name such a thing without also implying that something must be loaded from the internet.

@jfirebaugh
Copy link
Contributor

Another potential approach would be to make the initial request Required, have the cache miss reported as noContent regardless of necessity, and add some other flag to the response that allows the requester to distinguish between a cache hit or miss:

  1. Source makes an Required request for the ideal tile.
  2. If the cache hits, it's done.
  3. If the cache misses, it keeps the request open, but also makes Optional requests for successive parent tiles until either one of those is fulfilled, or the Required request for the ideal tile is fulfilled.

@kkaefer
Copy link
Member Author

kkaefer commented May 18, 2016

@jfirebaugh We need to transition the requests between Required and Optional state when zooming in/out, so in that case, we don't have a way to ensure that no HTTP request is made after transitioning from Required to Optional.

Introduces "optional" requests. These should be fulfilled by the FileSource only there's a low-cost/easy way to obtain the data (e.g. from a local cache). If the data for an optional request cannot be found, it *must* return a Response object with a NotFound error.

Traditional "required" requests still work the same way, with one change: If you set any prior* field in the Resource (i.e. priorModified, priorEtag, or priorExpires), the DefaultFileSource assumes that you already have the cache value and will not consult the cache before performing the request. If a prior cache lookup didn't turn up any data, and you therefore don't have an Etag or Modified value, you can still skip the cache by setting priorExpires. This will of course always result in a non-conditional HTTP request.
@kkaefer kkaefer force-pushed the 5027-optional-request branch from 25f5c4e to 8ee222c Compare May 18, 2016 10:40
@kkaefer kkaefer merged commit 8ee222c into master May 18, 2016
@jfirebaugh jfirebaugh deleted the 5027-optional-request branch June 3, 2016 23:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants