Skip to content

Store and update config metadata entries by namespace #7373

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

Merged
merged 8 commits into from
Feb 22, 2021

Conversation

karenyz
Copy link
Contributor

@karenyzkarenyz commented Jan 25, 2021

Currently fetch/activation of different namespaces will modify a shared set of metadata, however fields such as fetch_time, last_apply_time should be namespace-specific

Fixes#7179

Copy link
Contributor

@charlotteliangcharlotteliang left a comment

Choose a reason for hiding this comment

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

I was under the impression that the namespace API is deprecated in 3P concept. Do we know what are the use case or currently we are actually fetching per namespace and previous eng didn't manage to implement that in database?

@paulb777
Copy link
Member

  • @visumickey For input on addressing Performance's namespace needs.
@karenyz
Copy link
ContributorAuthor

I was under the impression that the namespace API is deprecated in 3P concept. Do we know what are the use case or currently we are actually fetching per namespace and previous eng didn't manage to implement that in database?

@chliangGoogle The use case is that Performance uses the namespace fireperf(initialized here), and calls fetchAndActivate independently of whether the developer uses RC. This should not affect the activation of the firebase template.

However right now activating fireperf will update the shared metadata(including last_apply_time) and this impacts activation of the firebase namespace ( based on last_apply_time it looks like the template has already been activated)

@paulb777
Copy link
Member

Should we consider migrating to a more flexible storage mechanism like GULUserDefaults? @chliangGoogle Do you know why sqlite was chosen to be used?

@charlotteliang
Copy link
Contributor

No particular reason using sqlite tbh, and I think it might be easier for @karenyz to migrate to GULUserDefaults with the new metadata design.

@google-oss-bot
Copy link

google-oss-bot commented Feb 19, 2021

Coverage Report

Affected SDKs

  • FirebaseRemoteConfig-iOS-FirebaseRemoteConfig.framework

    SDK overall coverage changed from 80.97% (144f23e) to 80.94% (cada4b2) by -0.02%.

    FilenameBase (144f23e)Head (cada4b2)Diff
    RCNConfigDBManager.m77.94%77.99%+0.05%
    RCNConfigSettings.m70.24%70.14%-0.09%

Test Logs

Copy link
Contributor

@charlotteliangcharlotteliang left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks for doing this!

@karenyzkarenyz merged commit 32922df into masterFeb 22, 2021
@karenyzkarenyz deleted the kz-perf-rc-fix branch February 22, 2021 18:21
ziadtamim pushed a commit to ziadtamim/firebase-ios-sdk that referenced this pull request Mar 11, 2021
* Insert and load config metadata by namespace * Deprecate existing metadata table name * Update tests * Fix delete method and update naming * Add test for writing metadate for multiple namespaces
@firebasefirebase locked and limited conversation to collaborators Mar 25, 2021
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.
4 participants
@karenyz@paulb777@charlotteliang@google-oss-bot
close