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

lib/model: Return empty summary on paused folders (ref #6272) #6326

Merged
merged 3 commits into from Feb 12, 2020

Conversation

@imsodin
Copy link
Member

imsodin commented Feb 11, 2020

This now actually addresses the 404 on the rest api that makes trouble for SyncTrayzor (https://forum.syncthing.net/t/rest-db-status-returns-404-on-startup/14329). The behaviour is, that we return an empty folder summary in that case. I think that's not a good API and instead the folder summary service should have access to the db for paused folders as well (e.g. global and local state are still useful info then), but that's a much bigger change. This just makes sure the behaviour doesn't change.

I didn't add a rest api error, as it can't use the model package. However I added a test in the model that makes sure that no error is returned when getting a folder summary for a paused folder.

errors, err := c.model.FolderErrors(folder)
if err == nil {
var snap *db.Snapshot
if snap, err = c.model.DBSnapshot(folder); err == nil {

This comment has been minimized.

Copy link
@calmh

calmh Feb 12, 2020

Member

release dat snap

This comment has been minimized.

Copy link
@calmh

calmh Feb 12, 2020

Member

Also, DBSnapshot still returns errFolderMissing for non-started folders does it not?

No, I guess that's if it's actually missing from the config, which is fine. 👍

This comment has been minimized.

Copy link
@AudriusButkevicius

AudriusButkevicius Feb 12, 2020

Member

The snap is released (end of the if block) if its acquired.

This comment has been minimized.

Copy link
@calmh

calmh Feb 12, 2020

Member

Now it is, yes, but it wasn't when I commented. :)

@calmh

This comment has been minimized.

Copy link
Member

calmh commented Feb 12, 2020

Needs merge

@imsodin imsodin force-pushed the imsodin:model/emptySummary branch from b84cfba to ea96eea Feb 12, 2020
@calmh
calmh approved these changes Feb 12, 2020
@calmh calmh merged commit ce27780 into syncthing:master Feb 12, 2020
12 of 13 checks passed
12 of 13 checks passed
GolangCI 1 issue found
Details
Build (Cross Compile) (Syncthing) TeamCity build finished
Details
Build (Debian, Snap) (Syncthing) TeamCity build finished
Details
Build (Discovery Server) TeamCity build finished
Details
Build (Linux) (Syncthing) TeamCity build finished
Details
Build (Linux, Go1.12) (Syncthing) TeamCity build finished
Details
Build (Linux, Go1.14-rc) (Syncthing) TeamCity build finished
Details
Build (Mac) (Syncthing) TeamCity build finished
Details
Build (Relay Server) TeamCity build finished
Details
Build (Source) (Syncthing) TeamCity build finished
Details
Build (Windows) (Syncthing) TeamCity build finished
Details
Check Correctness (Syncthing) TeamCity build finished
Details
Coverage (Syncthing) TeamCity build finished
Details
@calmh calmh added this to the v1.4.0 milestone Feb 12, 2020
@imsodin imsodin deleted the imsodin:model/emptySummary branch Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.