Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

Fix sample app to init Store on Application creation #149

Merged
merged 1 commit into from Mar 7, 2017
Merged

Fix sample app to init Store on Application creation #149

merged 1 commit into from Mar 7, 2017

Conversation

pavlospt
Copy link
Contributor

@pavlospt pavlospt commented Mar 7, 2017

Fixing #147 & #148

android:theme="@style/AppTheme">
<activity
android:name=".activity.PersistingStoreActivity">
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need both activities in manifest?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry that was just label duh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it's just a matter of what a user wants to test :)

@alexio alexio merged commit c8dce44 into nytimes:develop Mar 7, 2017
.fetcher(barCode -> provideRetrofit().fetchSubreddit(barCode.getKey(), "10"))
.memory(CacheBuilder.newBuilder()
.maximumSize(1)
.expireAfterWrite(10, TimeUnit.SECONDS)
Copy link

Choose a reason for hiding this comment

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

@pavlospt
The setting has no effect. The cache won't get cleared after 10 secs.
Still can get data from memory after 10 secs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know why, I'll fix it in am.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tsaisean it works fine for me. It didnt go through the fetcher during the 10 seconds period.

Copy link

Choose a reason for hiding this comment

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

@pavlospt What about after 10secs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tsaisean just saw that one. Since @digitalbuddha said he knows what's wrong, I think we should wait till he replies and someone can get on with a fix!

Copy link
Contributor

Choose a reason for hiding this comment

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

I need to double check but there's a few things working against this example. First there is a 1min denouncer in the network layer. Second there is a no op persister that is caching stuff in memory. Will have a fix today. Nothing should be affected for real cache policies (over 1 min)

Copy link
Contributor Author

@pavlospt pavlospt Mar 8, 2017

Choose a reason for hiding this comment

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

@digitalbuddha maybe we'll need to adjust the debouncer to the cache TTL ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes also the noop persister is currently backed by a hashmap, that also needs to be a guava cache with timeout same as memory cache and denouncer. So really denouncer and noop persister should have a max of the mem cache policy.

@pavlospt pavlospt deleted the fix_sample_app branch March 8, 2017 06:51
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.

None yet

4 participants