Skip to content

Conversation

@ArtificialOwl
Copy link
Member

restoring clearCachedConfig() that was removed in 25.0.0 with #32261

It can be useful in some use-case when config is edited by a parallel process

Signed-off-by: Maxence Lange <[email protected]>
@ArtificialOwl ArtificialOwl requested review from a team, blizzz, icewind1991 and juliusknorr and removed request for a team November 15, 2022 12:10
@ArtificialOwl
Copy link
Member Author

/backport to stable25

@ArtificialOwl ArtificialOwl added the 3. to review Waiting for reviews label Nov 15, 2022
@szaimen szaimen added this to the Nextcloud 26 milestone Nov 15, 2022
@PVince81 PVince81 requested a review from skjnldsv November 15, 2022 15:28
@juliusknorr
Copy link
Member

What exactly would be the use case there? If it is used in any app it probably should become a method on the public interface.

@ArtificialOwl
Copy link
Member Author

What exactly would be the use case there? If it is used in any app it probably should become a method on the public interface.

IAppConfig is not really kept up to date with AppConfig, but I totally agree that we should spent some time on this.

Now the use cases: oc_appconfig is perfect for any app that wants to store some values without creating it's own small table for the same result.
A cronjob or an interactive occ command might:

  • needs to cycle an heavy process,
  • wants to confirm a general settings/status from time to time.

(ie. this is used to avoid duplicate index by fulltextsearch)

If the app can refresh the cache of oc_appconfig before checking the status from time to time it will not need to create it's own table for the same result.

@juliusknorr
Copy link
Member

(ie. this is used to avoid duplicate index by fulltextsearch)

Sounds more like a case for https://github.com/nextcloud/server/blob/fcae6a68c347e2913cc29f45648be37789f09c29/lib/public/Lock/ILockingProvider.php

Now the use cases: oc_appconfig is perfect for any app that wants to store some values without creating it's own small table for the same result.
A cronjob or an interactive occ command might:
needs to cycle an heavy process,
wants to confirm a general settings/status from time to time.

While I know that is common behavior and used to also heavily make use of that, we might want to avoid this in the future due to the fact that all entries of oc_appconfig are loaded on every request. Maybe we could rather come up with a way to have a spearate config table for such things where entries are only queried from the DB when necessary - or introduce two types of values in oc_appconfig. Ones that are always loaded and others that apps need to request individually.

Would be good to see what @PVince81 thinks about that.

@juliusknorr
Copy link
Member

I'm not fully against getting this one back in, just that I think it might make sense to be careful how oc_appconfig is being (ab)used ;)

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍 as discussed before with @ArtificialOwl

@juliusknorr
Copy link
Member

Failure unrelated

@juliusknorr juliusknorr merged commit 884da19 into master Dec 19, 2022
@juliusknorr juliusknorr deleted the fix/noid/bringing-clear-config-back branch December 19, 2022 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants