Skip to content

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Mar 18, 2025

MAS tries to delete all the devices that it doesn't know about. This PR makes it ignore the dehydrated device (as reported by Synapse), so that it won't get deleted.

Companion PR: element-hq/synapse#18252

Part of a fix to element-hq/element-meta#2784

@uhoreg uhoreg force-pushed the ignore_dehydrated_devices branch from 550a615 to 20e2e24 Compare March 18, 2025 19:42
@richvdh richvdh requested a review from sandhose March 19, 2025 10:10
@richvdh richvdh added the Z-Build-Workflow Add this label to trigger a build workflow for this pull request label Mar 19, 2025
@sandhose sandhose added Z-Build-Workflow Add this label to trigger a build workflow for this pull request and removed Z-Build-Workflow Add this label to trigger a build workflow for this pull request labels Mar 19, 2025
@richvdh richvdh requested review from reivilibre and removed request for sandhose March 19, 2025 11:25
@reivilibre
Copy link
Contributor

Opens #4269

let existing_devices: HashSet<String> = body
.devices
.into_iter()
.filter(|d| d.dehydrated.is_none())
Copy link
Contributor

Choose a reason for hiding this comment

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

we could also alternatively consider hiding dehydrated devices from the admin API, but in principle I think it does make sense to show them there, so happy with this

Copy link
Contributor

Choose a reason for hiding this comment

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

that said, shouldn't we have d.dehydrated != Some(true) here, or make dehydrated: bool with a default of false? (probably in favour of the latter since that's the simplest and the semantics match)

Copy link
Contributor

Choose a reason for hiding this comment

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

though the struct is serialised again, so Option<bool> makes sense unless we split the usage — I'll change the filter condition, push, merge

let existing_devices: HashSet<String> = body
.devices
.into_iter()
.filter(|d| d.dehydrated.is_none())
Copy link
Contributor

Choose a reason for hiding this comment

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

that said, shouldn't we have d.dehydrated != Some(true) here, or make dehydrated: bool with a default of false? (probably in favour of the latter since that's the simplest and the semantics match)

@reivilibre reivilibre removed the Z-Build-Workflow Add this label to trigger a build workflow for this pull request label Mar 19, 2025
@reivilibre reivilibre enabled auto-merge March 19, 2025 12:36
@reivilibre reivilibre merged commit 6a9d4b4 into element-hq:main Mar 19, 2025
20 checks passed
@sandhose sandhose added A-Homeserver-Integration Integration with the homeserver T-Defect Something isn't working labels Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Homeserver-Integration Integration with the homeserver T-Defect Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants