Hi Does `flyte core` chart support sendgrid based `notificat Flyte #ask-the-community

Hi! Does `flyte-core` chart support sendgrid base...

Derek Yu

09/07/2023, 2:00 PM

Hi! Does

flyte-core

chart support sendgrid based

notifications

? Docs here say specify it can be configured for sandbox, but when I checked flyte-core template it seems to only support the AWS setup, and under a

workflow_notifications

section rather than

notifications

Haytham Abuelfutuh

09/07/2023, 3:54 PM

Hey @Derek Yu, yes it does. It's indeed under

workflow_notifications

in flyte-core (might be inconsistent) but you can put the same

emailer

block there and it will work just the same. Have you successfully tested it in sandbox?

Derek Yu

09/07/2023, 4:03 PM

No, not tested yet. Was just curious. Will test it out, thanks @Haytham Abuelfutuh

Derek Yu

09/08/2023, 4:48 PM

Hey @Haytham Abuelfutuh I tested. I tried using the

sendgrid

setup with

flyte-core

chart as such:

Copy code

workflow_notifications:
    enabled: true
    config:
      notifications:
        type: sandbox
        emailer:
          emailServerConfig:
            serviceName: sendgrid
            apiKeyEnvVar: $SENDGRID_API_KEY
          subject: "Notice: Execution \"{{ workflow.name }}\" has {{ phase }} in \"{{ domain }}\"."
          sender:  "<mailto:foo@foo.com|foo@foo.com>"
          body: >
             asdf

When upgrading the chart, this error appeared:

Copy code

helm.go:81: [debug] template: flyte/charts/flyte-core/templates/flytescheduler/deployment.yaml:16:27: executing "flyte/charts/flyte-core/templates/flytescheduler/deployment.yaml" at <include (print .Template.BasePath "/admin/configmap.yaml") .>: error calling include: template: flyte/charts/flyte-core/templates/admin/configmap.yaml:48:28: executing "flyte/charts/flyte-core/templates/admin/configmap.yaml" at <.Values.workflow_notifications.config.notifications.region>: wrong type for value; expected string; got interface {}
UPGRADE FAILED

Which suggests that the helm chart expects the fields for the

AWS setup

(region, publisher, processer) and not the

sendgrid

setup, which doesn't have those fields.

Derek Yu

09/08/2023, 4:51 PM

cc: @Yee as well, if you know anything, as saw you're also on this topic!

Derek Yu

09/11/2023, 7:29 PM

Hey bumping this, thanks!

Yee

09/11/2023, 7:29 PM

sorry in the middle of a ton of things… will respond later today

Haytham Abuelfutuh

09/12/2023, 12:10 AM

Hey @Derek Yu, have you attempted to set region?

Copy code

workflow_notifications:
    enabled: true
    config:
      notifications:
        type: sandbox
        region: wrong-region
        emailer:
          emailServerConfig:
            serviceName: sendgrid
            apiKeyEnvVar: $SENDGRID_API_KEY
          subject: "Notice: Execution \"{{ workflow.name }}\" has {{ phase }} in \"{{ domain }}\"."
          sender:  "<mailto:foo@foo.com|foo@foo.com>"
          body: >
             asdf

Haytham Abuelfutuh

09/12/2023, 12:10 AM

This error is coming from helm chart, I can dig up why it's throwing that but just want to see if we can get past this

Yee

09/12/2023, 1:59 AM

yeah what haytham said… that field ultimately should go here: https://github.com/flyteorg/flyte/blob/528313f762cf00efd51f12cda4836158e93ea6f6/charts/flyte-core/templates/admin/configmap.yaml#L48

Derek Yu

09/12/2023, 3:47 PM

Hey all thanks for the reply! I tried adding a dummy region and its able to deploy now 🙂 However

sendgrid

never attempts to send an email, and I don't see any evidence of flyteadmin trying to reach sendgrid in the logs (grepped on keywords

sendgrid

and

notification

) when running a successful execution with the following launchplan copied directly from the example in the docs:

Copy code

@task
def double_int_and_print(a: int) -> str:
    return str(a * 2)


@workflow
def int_doubler_wf(a: int) -> str:
    doubled = double_int_and_print(a=a)
    return doubled

wacky_int_doubler_lp = LaunchPlan.get_or_create(
    name="wacky_int_doubler",
    workflow=int_doubler_wf,
    default_inputs={"a": 4},
    notifications=[
        Slack(
            phases=[
                WorkflowExecutionPhase.SUCCEEDED,
                WorkflowExecutionPhase.ABORTED,
                WorkflowExecutionPhase.TIMED_OUT,
                WorkflowExecutionPhase.FAILED,
            ],
            recipients_email=["<http://xxx.slack.com|xxx.slack.com>"],
        ),
    ],
)

flyteadmin

I see the correct content passed to

/etc/flyte/config/notifications.yaml

Copy code

notifications:
  type: sandbox
  region: foo
  emailer:
    body: |
      Execution \"{{ workflow.name }} [{{ name }}]\" has {{ phase }} in \"{{ domain }}\". View details at <a href=\<https://xxx.xxxx.com/console/projects/{{> project }}/domains/{{ domain }}/executions/{{ name }}> <https://xxx.xxx.com/console/projects/{{> project }}/domains/{{ domain }}/executions/{{ name }}</a>. {{ error }}
    emailServerConfig:
      apiKeyEnvVar: xxxx
      serviceName: sendgrid
    sender: <mailto:myemail@xxx.com|myemail@xxx.com> (<- this email is my verified sender identity on sendgrid) 
    subject: 'Notice: Execution "{{ workflow.name }}" has {{ phase }} in "{{ domain }}".'

Is there anything else you recommend I check? Do these configurations looks correct? Thanks!

Derek Yu

09/14/2023, 12:26 PM

Is there someone I can pair with to debug? Still not sure why it's not hitting sendgrid

Yee

09/14/2023, 5:08 PM

Copy code

logger:
  show-source: true
  level: 5

Yee

09/14/2023, 5:08 PM

should be at the top level of a config yaml file read by admin

Yee

09/14/2023, 5:09 PM

https://github.com/flyteorg/flyteadmin/blob/master/pkg/async/notifications/implementations/gcp_processor.go

Derek Yu

09/14/2023, 5:10 PM

thanks for the help @Yee!

Derek Yu

09/18/2023, 6:50 AM

hey @Yee I wanted to follow up on what we discussed about the sandbox (sendgrid) implementation needing gcp pub sub. I checked and it seems like actually the processor and publisher for sandbox (see this PR) actually doesn't rely on any cloud pub sub? These were the only fields used in the notifications config (no specifications on publisher, processor, etc):

Copy code

notifications:
  type: sandbox
  emailer:
    emailServerConfig:
      apiKeyEnvVar: "SENDGRID_API_KEY"
    subject: "Notice: Execution \"{{ name }}\" has {{ phase }} in \"{{ domain }}\"."
    sender: "<mailto:example@gmail.com|example@gmail.com>"  # Consider using a generic email instead of personal
    body: >
      Execution \"{{ name }}\" has {{ phase }} in \"{{ domain }}\". View details at
      <a href=\<http://example.com/projects/{{> project }}/domains/{{ domain }}/executions/{{ name }}>
      <http://example.com/projects/{{> project }}/domains/{{ domain }}/executions/{{ name }}</a>. {{ error }}

Derek Yu

09/18/2023, 9:11 AM

I summarized the thread in an issue here

Yee

09/18/2023, 4:53 PM

i’m confused

Yee

09/18/2023, 4:53 PM

i thought you were doing this on gcp.

Yee

09/18/2023, 4:53 PM

not sandbox

Derek Yu

09/18/2023, 4:54 PM

Yeah no, I've been trying

type: sandbox

Yee

09/18/2023, 5:30 PM

i think you need gcp

Derek Yu

09/18/2023, 5:30 PM

I guess that was my question initially, was whether sandbox was a working implementation. So guessing not then?

Yee

09/18/2023, 5:31 PM

don’t rely on sandbox

Yee

09/18/2023, 5:31 PM

use pubsub

Yee

09/18/2023, 5:31 PM

you need to hit the gizmo library

Derek Yu

09/18/2023, 5:31 PM

ok got it, I'll try gcp next. Thanks

Yee

09/18/2023, 5:32 PM

before messing around with emailing, first confirm that you’re seeing messages come through just by looking at the gcp console

Derek Yu

09/18/2023, 5:33 PM

Also if sandbox is out of date, might be good to have the docs updated

Derek Yu

09/21/2023, 7:13 AM

After merging the helm fix, the next issue to arise is that the Published message seems to be missing the body, and the Processor (subscriber) cannot pull it. Opened an issue here

20 Views

Open in Slack

Previous Next