Context::set_config(): Restart IO scheduler if needed (#5111) #5117

iequidoo · 2023-12-20T04:00:52Z

See commit messages.

python/tests/test_1_online.py

src/config.rs

src/configure.rs

python/src/deltachat/testplugin.py

Hocuri · 2023-12-24T10:10:49Z

Note: Before we merge this, we should get some buy-in from UI people whether this actually makes their lives easier. Because we now have set_config, set_config_ex, set_raw_config, and set_config_inner, which does make reading the code harder. I personally never had any problem with needing to restart IO when writing UI code.

Or was there already a complaint from UI people about this?

link2xt · 2023-12-24T11:09:45Z

Note: Before we merge this, we should get some buy-in from UI people whether this actually makes their lives easier. Because we now have set_config, set_config_ex, set_raw_config, and set_config_inner, which does make reading the code harder. I personally never had any problem with needing to restart IO when writing UI code.

This is a bit ugly, but the only external API is still set_config.

Or was there already a complaint from UI people about this?

Android does it manually, it can be removed from Android then. JSON-RPC bindings already do this inside the bindings, now it is moved to the core.

There was no complaint from UI devs, but @missytake got a problem with the bot: #5111
This is not a nice API for writing the bots if changing the setting after configuring silently breaks delivery because messages are moved to DeltaChat folder but not downloaded.

Hocuri · 2023-12-25T19:24:39Z

There was no complaint from UI devs, but @missytake got a problem with the bot: #5111
This is not a nice API for writing the bots if changing the setting after configuring silently breaks delivery because messages are moved to DeltaChat folder but not downloaded.

OK then it's nice that we fix this footgun 👍

iequidoo · 2023-12-26T20:24:27Z

I also don't like that now we have so much set_config*() stuff, set_config_internal() is just a syntactic sugar, i see no difference with just writing set_config_ex(Sync, ...) or set_config_ex(None, ...). But it doesn't matter much

src/imap.rs

iequidoo · 2023-12-28T02:51:34Z

And now this is the problem:
9.16 [events-ac2] INFO src/imap.rs:1873: got unsolicited response Other(ResponseData { raw: 4096, response: Data { status: Ok, code: Some(CopyUid(1703729259, [Uid(11)], [Uid(1)])), information: Some("Moved UIDs.") } })
"DeltaChat" needs to be fetched again if so.

Added another one fix for this. But not quite sure the server behaviour is correct.

EDIT: So, the tests passed 3 times w/o failures, apparently two extra fixes are working. But it's weird that several footguns were found while implementing such a simple thing.

iequidoo · 2024-02-06T04:05:57Z

Now only test_verified_group_vs_delete_server_after() failed which is expectable

iequidoo · 2024-02-06T20:25:18Z

Moved a couple of fixes related to mvbox creation to #5251 for easier review.

EDIT: Can't easily split out fix: Create mvbox on setting mvbox_move to a separate review because of the text conflicts, so i suggest to review the PR as is.

src/config.rs

Restart the IO scheduler if needed to make the new config value effective (for `MvboxMove, OnlyFetchMvbox, SentboxWatch` currently). Also add `set_config_internal()` which doesn't affect running the IO scheduler. The reason is that `Scheduler::start()` itself calls `set_config()`, although not for the mentioned keys, but still, and also Rust complains about recursive async calls.

this is done by core as needed since deltachat/deltachat-core-rust#5117

iequidoo commented Dec 20, 2023

View reviewed changes

python/tests/test_1_online.py Outdated Show resolved Hide resolved

iequidoo marked this pull request as ready for review December 20, 2023 04:39

iequidoo requested a review from link2xt December 20, 2023 04:39

link2xt reviewed Dec 21, 2023

View reviewed changes

src/config.rs Outdated Show resolved Hide resolved

iequidoo force-pushed the iequidoo/apply_config branch 2 times, most recently from 7454397 to c4325a0 Compare December 21, 2023 22:48

iequidoo changed the title ~~feat: Add Context::apply_config() and switch to it in public APIs (#5111)~~ feat: Context::set_config(): Restart IO scheduler if needed (#5111) Dec 21, 2023

iequidoo marked this pull request as draft December 22, 2023 00:32

link2xt reviewed Dec 22, 2023

View reviewed changes

src/configure.rs Outdated Show resolved Hide resolved

iequidoo force-pushed the iequidoo/apply_config branch 2 times, most recently from 5694c87 to ed2636f Compare December 22, 2023 23:39

iequidoo marked this pull request as ready for review December 22, 2023 23:46

iequidoo requested review from link2xt and Simon-Laux December 22, 2023 23:46

This comment was marked as resolved.

Sign in to view

iequidoo force-pushed the iequidoo/apply_config branch from ed2636f to 9cd2b4f Compare December 23, 2023 03:33

iequidoo commented Dec 23, 2023

View reviewed changes

python/src/deltachat/testplugin.py Outdated Show resolved Hide resolved

iequidoo force-pushed the iequidoo/apply_config branch from 9cd2b4f to 5cd851e Compare December 26, 2023 20:17

iequidoo requested a review from link2xt December 26, 2023 20:32

iequidoo force-pushed the iequidoo/apply_config branch from 5cd851e to 287cfaf Compare December 27, 2023 03:01

iequidoo marked this pull request as draft December 27, 2023 03:26

iequidoo force-pushed the iequidoo/apply_config branch from 287cfaf to fb6a1f2 Compare December 28, 2023 01:57

iequidoo commented Dec 28, 2023

View reviewed changes

src/imap.rs Outdated Show resolved Hide resolved

iequidoo marked this pull request as ready for review December 28, 2023 02:02

iequidoo force-pushed the iequidoo/apply_config branch from 01dbee4 to 8bb1dce Compare December 28, 2023 03:50

iequidoo force-pushed the iequidoo/apply_config branch 6 times, most recently from 9328c39 to b9673ff Compare February 6, 2024 03:44

iequidoo marked this pull request as ready for review February 6, 2024 04:12

iequidoo marked this pull request as draft February 6, 2024 19:58

iequidoo force-pushed the iequidoo/apply_config branch from b9673ff to 5d0e9cc Compare February 6, 2024 20:03

iequidoo changed the base branch from main to iequidoo/create-mvbox February 6, 2024 20:04

iequidoo marked this pull request as ready for review February 6, 2024 20:05

iequidoo changed the title ~~Context::set_config(): Restart IO scheduler if needed + related fixes (#5111)~~ Context::set_config(): Restart IO scheduler if needed (#5111) Feb 6, 2024

Base automatically changed from iequidoo/create-mvbox to main February 9, 2024 02:39

iequidoo force-pushed the iequidoo/apply_config branch from 5d0e9cc to b896eff Compare February 9, 2024 03:46

link2xt approved these changes Feb 12, 2024

View reviewed changes

src/config.rs Outdated Show resolved Hide resolved

iequidoo added 3 commits February 12, 2024 14:24

feat: server_sent_unsolicited_exists(): Log folder name

a768e95

fix: Create mvbox on setting mvbox_move

f1d1347

iequidoo force-pushed the iequidoo/apply_config branch from b896eff to f1d1347 Compare February 12, 2024 17:26

link2xt approved these changes Feb 12, 2024

View reviewed changes

iequidoo merged commit 9933a42 into main Feb 12, 2024
38 checks passed

iequidoo deleted the iequidoo/apply_config branch February 12, 2024 18:41

r10s added a commit to deltachat/deltachat-ios that referenced this pull request Feb 14, 2024

start/stop IO no longer needed on config changes

756f562

this is done by core as needed since deltachat/deltachat-core-rust#5117

r10s mentioned this pull request Feb 14, 2024

start/stop IO no longer needed on config changes deltachat/deltachat-ios#2076

Merged

r10s added a commit to deltachat/deltachat-android that referenced this pull request Feb 14, 2024

start/stop IO no longer needed on config changes

98f65cf

this is done by core as needed since deltachat/deltachat-core-rust#5117

r10s mentioned this pull request Feb 14, 2024

start/stop IO no longer needed on config changes deltachat/deltachat-android#2959

Merged

r10s added a commit to deltachat/deltachat-android that referenced this pull request Feb 14, 2024

start/stop IO no longer needed on config changes (#2959)

965021d

this is done by core as needed since deltachat/deltachat-core-rust#5117

r10s added a commit to deltachat/deltachat-ios that referenced this pull request Feb 14, 2024

start/stop IO no longer needed on config changes (#2076)

bba12dc

this is done by core as needed since deltachat/deltachat-core-rust#5117

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Context::set_config(): Restart IO scheduler if needed (#5111) #5117

Context::set_config(): Restart IO scheduler if needed (#5111) #5117

iequidoo commented Dec 20, 2023 •

edited

Loading

This comment was marked as resolved.

Hocuri commented Dec 24, 2023

link2xt commented Dec 24, 2023

Hocuri commented Dec 25, 2023

iequidoo commented Dec 26, 2023

iequidoo commented Dec 28, 2023 •

edited

Loading

iequidoo commented Feb 6, 2024

iequidoo commented Feb 6, 2024 •

edited

Loading

Context::set_config(): Restart IO scheduler if needed (#5111) #5117

Context::set_config(): Restart IO scheduler if needed (#5111) #5117

Conversation

iequidoo commented Dec 20, 2023 • edited Loading

This comment was marked as resolved.

Hocuri commented Dec 24, 2023

link2xt commented Dec 24, 2023

Hocuri commented Dec 25, 2023

iequidoo commented Dec 26, 2023

iequidoo commented Dec 28, 2023 • edited Loading

iequidoo commented Feb 6, 2024

iequidoo commented Feb 6, 2024 • edited Loading

iequidoo commented Dec 20, 2023 •

edited

Loading

iequidoo commented Dec 28, 2023 •

edited

Loading

iequidoo commented Feb 6, 2024 •

edited

Loading