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

[core] Add style::Source::setVolatile()/isVolatile() API#16422

Merged
pozdnyakov merged 8 commits intomasterfrom
mikhail_volatile_source
Apr 22, 2020
Merged

[core] Add style::Source::setVolatile()/isVolatile() API#16422
pozdnyakov merged 8 commits intomasterfrom
mikhail_volatile_source

Conversation

@pozdnyakov
Copy link
Copy Markdown
Contributor

The tile data from the volatile sources are not stored in local storage, i.e. they are not cached.

Fixes https://github.com/mapbox/mapbox-gl-native-team/issues/335

@pozdnyakov pozdnyakov changed the title Mikhail volatile source [core] Add style::Source::setVolatile()/isVolatile() API Apr 21, 2020
}

void DatabaseFileSource::forward(const Resource& res, const Response& response, std::function<void()> callback) {
if (res.storagePolicy == Resource::StoragePolicy::Volatile) return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also have something like this when requesting volatile resources from the offline database. If they are never stored there is no point in querying for them.

@tmpsantos
Copy link
Copy Markdown
Contributor

Kudos, this was very clean! Looks like the bots got stuck because of the GitHub outage. :-/

std::string getID() const;
optional<std::string> getAttribution() const;

// The data from the volatile sources are not stored in a persistent storage.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@pozdnyakov thanks for the quick turnaround on this. Implementation looks great. Do we know how likely we are to introduce additional storage options for sources in the future?

From an API extensibility perspective, I'm wondering if it would be better to introduce smth like Source::getStoragePolicy and Source::setStoragePolicy. If we were to introduce new storage types alongside volatile and permanent in the future, we wouldn't need to create new APIs and the boolean flags wouldn't need to be deprecated.

cc @tmpsantos @alexshalamov @LukasPaczos @tobrun @1ec5 @julianrex

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@chloekraw the Carbon API for sources uses generic parameters, so changing from a boolean flag to a enumeration type in future shall be painless 🙂

@pozdnyakov pozdnyakov force-pushed the mikhail_volatile_source branch from 69ca9d2 to b4fa87b Compare April 22, 2020 10:49
Comment thread platform/default/src/mbgl/storage/database_file_source.cpp Outdated
@pozdnyakov pozdnyakov force-pushed the mikhail_volatile_source branch from b4fa87b to 784d569 Compare April 22, 2020 11:15
@pozdnyakov pozdnyakov force-pushed the mikhail_volatile_source branch from 784d569 to f01e2f4 Compare April 22, 2020 13:24
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.

4 participants