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

chore(platform): Extract relay into separate services #2823

Merged
merged 10 commits into from
Jan 28, 2024

Conversation

betimshahini
Copy link
Contributor

@betimshahini betimshahini commented Jan 19, 2024

Description

Splits the relay logic into a singleton distributor service and env-specific processor services. Creates new-patterned masked emails for existing masks, if they get re-authorized.

  • Secrets and vars set in all envs in GHA
  • Email rules set up
  • GHA workflows included

Related Issues

Testing

  • Create new masked email
  • Send email to it
  • Receive email in masked source address

Checklist

  • I have read the CONTRIBUTING guidelines
  • I have tested my code (manually and/or automated if applicable)
  • I have updated the documentation (if necessary)

@betimshahini betimshahini force-pushed the chore/2790-extract-relay-to-separate-worker branch from 4009b81 to 88c8890 Compare January 19, 2024 20:38
@betimshahini betimshahini marked this pull request as ready for review January 20, 2024 00:18
Copy link
Contributor

@szkl szkl left a comment

Choose a reason for hiding this comment

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

I'd like to consider adding email prefix to the relay workers.

The message forwarding is a neat trick probably taking care of a few
things along the way.

Comment on lines 40 to 42
throw new BadRequestError({
message: `Could not find hidden address ${input.maskedEmail}`,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Could it be a NotFoundError instead if the masked email doesn't have
a source account? The reason is any masked email account must have a
source account. It'd mean that masked address actually doesn't exist
in this case.

Comment on lines 12 to 18
"scripts": {
"build": "wrangler publish --dry-run --outdir=dist",
"test": "echo \"Error: no test specified\" && exit 0",
"dev:wrangler": "wrangler dev ",
"dev": "run-p 'dev:*'",
"deploy": "wrangler publish"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be better to have same set of utility scripts for checks.

) as [string, string][]

const envKeyPair = distEmailEntries.filter(
([distributorEnvPrefix, _]) => distributorEnvPrefix === envPrefix
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know about array destructuring in arrow function parameter
list. Nice.

Comment on lines 38 to 55
const sourceAccountURN = await node.storage.get<AccountURN>('source-account')
if (!sourceAccountURN)
throw new BadRequestError({
message: `Could not find hidden address ${input.maskedEmail}`,
})

const sourceAccountNode = initAccountNodeByName(
sourceAccountURN,
ctx.env.Account
)

const name = (await sourceAccountNode.class.getNickname()) || ''
const address = await sourceAccountNode.class.getAddress()

if (!address)
throw new InternalServerError({
message: `Could not find source address for masked email ${input.maskedEmail}`,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to make this a wrapper for
account.getSourceAccount procedure with implementing additional
checks that might be missing in getSourceAccount and
EmailAccount.getSource methods.

@szkl
Copy link
Contributor

szkl commented Jan 24, 2024

This would be a breaking change, right?

szkl
szkl previously approved these changes Jan 25, 2024
@szkl szkl merged commit 5ca7b8c into main Jan 28, 2024
16 checks passed
@szkl szkl deleted the chore/2790-extract-relay-to-separate-worker branch January 28, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore(platform): Extract email relay to separate worker
2 participants