Repository navigation
Conversation
2deae58 to
6ad0f04
Compare
46acbc7 to
494be7f
Compare
6ad0f04 to
afca961
Compare
494be7f to
9b3f6a6
Compare
4fc4ff3 to
4750068
Compare
7c0aec6 to
233c2cb
Compare
ae10a5e to
240121c
Compare
240121c to
4acf71d
Compare
This comment was marked as low quality.
This comment was marked as low quality.
|
@pbuzdin please create a separate account for your bot and don't let it post any comments in any chatmail or deltachat organization repositories. There are some community standards at https://delta.chat/en/community-standards, but even if they were not written down, it should be obvious that it is not great to run a bot that posts comments here pretending to be a human and then note that "a first-class preferred-transport concept has been requested there (comment 6090501441)" in pbuzdin/velta#114 (comment) |
|
That wasnt a bot comment. I wrote it myself because I'm having trouble using Deltachat in my region without manually selecting a relay for sending. |
|
our idea for situation "one relay cannot send" is to improve sending failover. or, in the future, replacing that relay automatically. adding more options, that most ppl would not understand, is often not a solution from our view. a configured_addr or used_for_sending were always meant as temporary things since we introduced multi relay. not having tons of options is also easier for us to debug and to get the model. nerds detecting smth is wrong with a relay, by some log or so, can remove it. as a general rule of thumb, we avoid options and beforehand questions whereever we can. btw: the screenshots of velta look great! |
e073767 to
f25c3ac
Compare
f25c3ac to
0059e8d
Compare
0059e8d to
ea7e2bc
Compare
Hocuri
left a comment
There was a problem hiding this comment.
deltachat.hneeds to be updated in order to remove theconfigured_addrreference.- Changelog entry should be a bit clearer, in case people are still setting or getting this config, e.g.
refactor: remove `configured_addr` configor evenrefactor!: remove `configured_addr` config
FTR:
- The
configured_addrconfig which is removed in this PR was mainly used in tests, only very few places in the actual code still used it anymore - This does keep
get_primary_self_addr(), which might be renamed intoget_any_self_addr()orget_some_self_addr()because there is nothing special about this addr anymore - We still have the
Addrconfig; we have been keeping it for bots that still setaddrandmail_pwfor configuration, but at some point we'll need to look for all bots that use it, make PRs to use a new API, and then remove theAddrconfig. (@link2xt correct me if I'm wrong)
| log_sink: LogSink, | ||
|
|
||
| /// From address to be used when emulating sending. | ||
| from: Option<String>, |
There was a problem hiding this comment.
I'm wondering if we should call this something like from_for_test_msgs, in order to make clear that it's only used in tests (which isn't obvious on all caller sites otherwise). But it's fine as-is, too.
| let configured_addr: String = transaction.query_row( | ||
| "SELECT value FROM config WHERE keyname='configured_addr'", | ||
| (), | ||
| |row| row.get(0), | ||
| )?; |
There was a problem hiding this comment.
This statement errors if there is no configured_addr. This means that a database created by a new Delta Chat can't be imported into an old Delta Chat.
Bumping DBVERSION would mean that we need coordinated releases on all platforms again, so, we likely want to avoid this. So, unless we find another solution, we likely have to keep some code that sets and updates configured_addr, and then remove it in some future version.
Concretely, after every transport mutation (e.g. save_transport, delete_transport, sync_transports), run INSERT OR REPLACE INTO config (keyname, value) SELECT 'configured_addr', addr FROM transports ORDER BY id LIMIT 1. Then add a test that creates a profile, deletes the first transport, and asserts the row points at an existing transport.
There was a problem hiding this comment.
I added a failing cross-core test now (test_cross_core.py::test_old_device_delete_transport). It sets up old second device and tries to delete a transport.
There was a problem hiding this comment.
Fixed it now by updating configured_addr with new transport::update_configured_addr in several places.
| contacts = ac2.get_contacts() | ||
| assert len(contacts) == 1 | ||
| contact2 = contacts[0] | ||
| assert contact2.addr == ac_contact.get_config("configured_addr") |
There was a problem hiding this comment.
Could use list_transports()[0]["addr"] rather than removing this line, but it's fine as-is, too (same for the next removed line below)
There was a problem hiding this comment.
This is a CFFI test, so we don't have list_transports(), but i have added assert contact2.addr == ac_contact.get_self_contact().addr instead.
| [transport1, transport2] = ac1_transports | ||
| ac1_clone.wait_for_event(EventType.TRANSPORTS_MODIFIED) | ||
| assert len(ac1_clone.list_transports()) == 2 | ||
| assert ac1_clone.get_config("configured_addr") == transport1["addr"] |
There was a problem hiding this comment.
This test now doesn't test a lot anymore, it only does some very basic things:
- ac1 and ac1_clone is a multi-device profile
- ac1 adds a transport
- ac1 sends a message, the test doesn't control which transport is used for this
- Bob answers
- ac1_clone receives the answer
Until we have functional end-to-end tests that can control the network (i.e. a test that can block the first transport in order to ensure that the message is sent via the second transport), I guess we can't do much here, but we need to update the test name and comments. And then we need to make sure to quickly make functional bad-network tests possible, because right now we wouldn't notice any regressions in this area until DC is deployed.
We can also remove |
ea7e2bc to
e51fc0e
Compare
e51fc0e to
a101a2f
Compare
2b5482f to
4d19d35
Compare
8a5f445 to
0e81c0d
Compare
a4925a2 to
33c2c81
Compare
configured_addr config
… interrupt channel is closed Without connection candidates IMAP busy loops when you try to stop I/O because interrupt channel gets closed and recv() always returns an error immediately, but no connection is attempted afterwards so IMAP loop keeps calling .prepare() in a loop without ever yielding control to executor, so it cannot even be cancelled. New JSON-RPC test test_pseudo_transport_start_stop_io reproduces the problem from CFFI test tests/test_3_offline.py::TestOfflineChat::test_send_lots_of_offline_msgs which added one transport without connection candidates and started I/O.
configured_addr was already not used by real SMTP code. Rust tests which emulate sending with TestContext.pop_sent_msg() now have a way to set the "sending" address manually in memory.
0e81c0d to
6a3216e
Compare
33c2c81 to
de8c2fd
Compare
configured_addr was already not used by real SMTP code.
Rust tests which emulate sending with TestContext.pop_sent_msg()
now have a way to set the "sending" address manually in memory.
Closes #8572