sentry icon indicating copy to clipboard operation
sentry copied to clipboard

Ignore set on disk options as drift

Open kneeyo1 opened this issue 1 year ago • 1 comments

In Sync mode, we do not want options that are not called in configoptions but set_on_disk to show up as drift.

Some options have the prioritize_disk flag and have values on the db and the disk. configoptions should only update these options if they are not set_on_disk. Right now, if it is set_on_disk but also set in the db, we write it as drift. This PR adds a check for that.

kneeyo1 avatar May 13 '24 21:05 kneeyo1

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.02%. Comparing base (bef6fe8) to head (347dccc). Report is 74 commits behind head on master.

:exclamation: Current head 347dccc differs from pull request most recent head a034c80. Consider uploading reports for the commit a034c80 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #70811   +/-   ##
=======================================
  Coverage   80.01%   80.02%           
=======================================
  Files        6505     6505           
  Lines      290769   290764    -5     
  Branches    50112    50118    +6     
=======================================
  Hits       232670   232670           
+ Misses      57662    57657    -5     
  Partials      437      437           

see 18 files with indirect coverage changes

codecov[bot] avatar May 13 '24 22:05 codecov[bot]

I think I am missing something here. Are you saying that we have options that are set on disk and also on the DB to a value that is not the default value for the option?

This seems something that should not happen as we do not allow an option to be updated on DB if it is set on disk. This could only have happened if the option was already set to a value on the DB (different than the default) and then set on disk to something different again.

But in that case it means the option cannot be updated with any channel, we should not hide this scenario which is broken.

We have options that are set on disk. In the db, they are set to their default value. This means that they exist on the DB, but do not read from it, they are still reading the disk value (which is not a default value).

I don't think this behavior is broken, as FLAG_PRIORTIZE_DISK seems to do exactly as said, take the disk value if it exists, otherwise read from the db.

Previously, when configoptions sync is called, for all options that exist and are set (even if just set to their default value), if they were not last updated by the automator, we write it as drift.

My change just checks if an option is already set on disk, if it is, don't write it as drift.

kneeyo1 avatar May 14 '24 21:05 kneeyo1