Skip to content
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

Incorrect rescan interval on auto accepted encrypted folder #8572

Closed
Ratio2 opened this issue Oct 2, 2022 · 4 comments · Fixed by #8573
Closed

Incorrect rescan interval on auto accepted encrypted folder #8572

Ratio2 opened this issue Oct 2, 2022 · 4 comments · Fixed by #8573
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion needs-triage New issues needed to be validated
Milestone

Comments

@Ratio2
Copy link
Contributor

Ratio2 commented Oct 2, 2022

Version: main

Rescan interval always change to 1 day, but gui only change 1 minute and 1 hour values to 1 day.

@Ratio2 Ratio2 added bug A problem with current functionality, as opposed to missing functionality (enhancement) needs-triage New issues needed to be validated labels Oct 2, 2022
@acolomb
Copy link
Member

acolomb commented Oct 2, 2022

I believe this works as designed? When accepting in the GUI, the same default rescan interval of 86400 seconds is preset, which you can change before saving though.

Doing the same for auto-accepts was added recently in #8427. Since the user has no chance to adjust it, we simply use the default value. Note that rescanning is usually not necessary for receive-encrypted folders, as the locally encrypted files can not be edited reasonably.

@Ratio2
Copy link
Contributor Author

Ratio2 commented Oct 2, 2022

@acolomb No, interval for encrypted folder not changed in gui if we set 61 or 3601 in default folder configuration

My fix only replicate gui behavior when user add folder manually.

@acolomb
Copy link
Member

acolomb commented Oct 3, 2022

Right, the GUI code assumes that the interval should only change automatically with the other settings (watcher enabled / folder type) when it's value is still among the three possible defaults. Duplicating this logic in the backend for the auto-accept case is error-prone, as we can see there is already a slight difference in behavior.

This functionality predates the possibility to set defaults for the folder config, so I guess we really need to define what the desired behavior is. If someone deliberately sets the default interval to one day for example, it will be changed back to a minute or an hour depending on the folder type / watcher setting. That is unexpected and not transparent. Maybe we can come up with an approach to fix that as well, then mirror it to auto-accept?

Ratio2 added a commit to Ratio2/syncthing that referenced this issue Oct 3, 2022
@Ratio2
Copy link
Contributor Author

Ratio2 commented Oct 3, 2022

I have taken your comments and made the necessary changes. These changes are minimal and do not require deepening the discussion. Now the scan interval cannot be less than the one set by the user. As a result, we have the following cases (the functionality has been tested):

  1. Scan interval 0 - we use 0, since there is no point in scanning an encrypted folder if we do not scan a regular folder
  2. Scan interval is less than 1 day - use 1 day to keep the current behavior
  3. Scanning interval is greater than 1 day - we use the user setting, since it makes no sense to use a value less for an encrypted folder than for a regular folder

Note: pull-request auto checks failed because the problem is in the server, not in the code

Ratio2 added a commit to Ratio2/syncthing that referenced this issue Oct 4, 2022
imsodin pushed a commit that referenced this issue Dec 21, 2022
calmh added a commit to calmh/syncthing that referenced this issue Dec 21, 2022
* upstream/main:
  lib/fs: Watching is unsupported on android/amd64 (fixes syncthing#8709) (syncthing#8710)
  lib/model: Only log at info level if setting change time fails (syncthing#8725)
  lib/model: Don't lower rescan interval from default on auto accepted enc folder (fixes syncthing#8572) (syncthing#8573)
  gui, man, authors: Update docs, translations, and contributors
  gui: Remove unmaintained language variant nl-BE (syncthing#8722)
  gui, script: Fix indentation in lang-en.json to match others (syncthing#8721)
  docker: Ensure entrypoint is executable (syncthing#8719)
  Go 1.19.4
  Update dependencies (syncthing#8717)
  gui, man, authors: Update docs, translations, and contributors
@calmh calmh added this to the v1.22.3 milestone Dec 27, 2022
calmh added a commit to calmh/syncthing that referenced this issue Jan 23, 2023
* main: (69 commits)
  Handle relay connect timeout (fixes syncthing#8749) (syncthing#8755)
  gui, man, authors: Update docs, translations, and contributors
  build: Go 1.19.5
  gui, man, authors: Update docs, translations, and contributors
  script: Add weblatedl.go for downloading updated translations (syncthing#8723)
  gui: Allow to translate action and type in Recent Changes modal (syncthing#8548)
  gui, man, authors: Update docs, translations, and contributors
  gui: Fix undefined lastSeenDays error in disconnected-inactive status check (ref syncthing#8530) (syncthing#8730)
  gui, man, authors: Update docs, translations, and contributors
  gui, api: Indicate running under container (syncthing#8728)
  lib/fs: Use io/fs errors as recommended in std lib (syncthing#8726)
  build: Handle co-authors (ref syncthing#3744) (syncthing#8708)
  lib/fs: Watching is unsupported on android/amd64 (fixes syncthing#8709) (syncthing#8710)
  lib/model: Only log at info level if setting change time fails (syncthing#8725)
  lib/model: Don't lower rescan interval from default on auto accepted enc folder (fixes syncthing#8572) (syncthing#8573)
  gui, man, authors: Update docs, translations, and contributors
  gui: Remove unmaintained language variant nl-BE (syncthing#8722)
  gui, script: Fix indentation in lang-en.json to match others (syncthing#8721)
  docker: Ensure entrypoint is executable (syncthing#8719)
  Go 1.19.4
  ...
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Dec 22, 2023
@syncthing syncthing locked and limited conversation to collaborators Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion needs-triage New issues needed to be validated
Projects
None yet
4 participants