Rebase

Rebase

Rebase

Rebase

Hmm... to me, I think it would be much more simple to have the components generate a DMOp, and then to have a utility function that calls processDMOperation on it, and put the result in a Redux action. That way, we avoid having to support multiple different kinds of actions. What do you think?

Yeah, agree, it's simpler. It's similar to an updated solution I was thinking about. But there are some things we have to figure out:

1. As of D12780 processDMOperation returns rawMessageInfos and updateInfos. But we also need to generate OutboundP2PMessages (generateMessagesToPeers function from this diff) - we probably should do that as a part of processDMOperation.
2. We need to know the sending status of messages that aren't automatically retriable. We can have a function that takes OutboundP2PMessages and returns a promise that can be passed to dispatchActionPromise.
3. Manual retries need IDs of failed messages - that is something relevant only for local messages. But we will need to modify them in a way that allows storing the IDs.

Figured sharing these makes us sure we're on the same page.

@bartek thanks for catching that

remove copy of keyserver_secondary.tf from root

Use boolean value

Rebase

Return boolean value

It might be a good idea to test both insert and update.

This diff looks confusing to me. We're introducing a new context PeerOlmSessionCreatorContext but it doesn't look like we're using it.

In D12730#360482, @ashoat wrote:

@will mentioned a concern I had raised in a 1:1 here. Not sure if we want to address that here, but broadly I'm worried that the health check might not work if we're doing it after bringing the load balancer publicly online with only a single node (the primary) accessible

Adding @varun as blocking to answer the inline comment

Heads-up that I landed a different migration 48 in D12759, so you'll have to rebase this one. Sorry about that!

Heads-up that I landed a different migration 48 in D12759, so you'll have to rebase this one. Sorry about that!

feedback. remove unnecessary homebrew install deps script

review feedback

rebase

only create vpc, subnets, and internet gateway if user created option is true

feedback

reduce line count

include logic to disable traffic until all nodes available

create secondary resources after primary

typo

include security group

What do you mean by business logic? Is it the logic that lives in components, or in reducers, or maybe somewhere else?

In D12776#361491, @ashoat wrote:

Can you provide some more context about this? I don't quite understand why it makes sense to start from an action... my initial thinking was that business logic would construct a DMOperation directly, rather than constructing an action that gets converted into a DMOperation

Can you provide some more context about this? I don't quite understand why it makes sense to start from an action... my initial thinking was that business logic would construct a DMOperation directly, rather than constructing an action that gets converted into a DMOperation

Right now two JS threads (shared worker and service worker) can concurrently modify the same key in IndexedDB. I think to make it safe we should use transactions to make sure this is safe.

Remove TODO

Fix race condition by exposing olm session creation via context

In D12668#361372, @kamil wrote:

I really don't like the idea (looking at this diff and the entire stack) of passing isKeyserverSession - notification crypto module should treat all devices the same regardless of whether this is a keyserver or client device, and we should only have senderID, because the current design makes it confusing. But given this is implemented and tested I believe we can proceed, you could create a follow-up task to improve it in future but for now LGTM.

Ah okay, no need to create a new follow-up task then – thanks for linking!

The HTTP request-response logic looks good; it's analogous to the existing Blob GET request logic.

Rename

Fix indentation

Rename column

I really don't like the idea (looking at this diff and the entire stack) of passing isKeyserverSession - notification crypto module should treat all devices the same regardless of whether this is a keyserver or client device, and we should only have senderID, because the current design makes it confusing. But given this is implemented and tested I believe we can proceed, you could create a follow-up task to improve it in future but for now LGTM.