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.

Sign up for GitHub

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

Jump to bottom

notify_slack should determine dag_id on its own #4884

Open
AetherUnbound opened this issue Sep 6, 2024 · 4 comments · May be fixed by #5136
Open

notify_slack should determine dag_id on its own #4884

AetherUnbound opened this issue Sep 6, 2024 · 4 comments · May be fixed by #5136
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: catalog Related to the catalog and Airflow DAGs 🔧 tech: airflow Involves Apache Airflow

Comments

@AetherUnbound
Copy link
Collaborator

Description

Since common.slack.notify_slack is an Airflow TaskFlow API @task, it has access to the context variables: https://airflow.apache.org/docs/apache-airflow/stable/core-concepts/taskflow.html#context

Thus, we should be able to have it determine the dag_id on its own (via the dag.dag_id property of the dag object available for templating).

This function will need to be changed to remove the dag_id required parameter from the outer function call, add dag: DAG | None = None at the end of the argument list, and then pass dag.dag_id into the send_message call:

openverse/catalog/dags/common/slack.py

Lines 421 to 433 in 7f5baa2

@task
def notify_slack(
text: str,
dag_id: str,
username: str = "Airflow Notification",
icon_emoji: str = ":airflow:",
) -> None:
send_message(
text,
username=username,
icon_emoji=icon_emoji,
dag_id=dag_id,
)

@AetherUnbound AetherUnbound added good first issue New-contributor friendly help wanted Open to participation from the community 💻 aspect: code Concerns the software code in the repository 🔧 tech: airflow Involves Apache Airflow 🟩 priority: low Low priority and doesn't need to be rushed 🧰 goal: internal improvement Improvement that benefits maintainers, not users 🧱 stack: catalog Related to the catalog and Airflow DAGs labels Sep 6, 2024
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Sep 6, 2024
@triii10
Copy link

triii10 commented Sep 8, 2024

Hi, I'm new to contributing to public OpenSource projects and would like to take this up if available.

@AetherUnbound
Copy link
Collaborator Author

Hi @triii10 , thank you for your interest in contributing to Openverse! I've assigned this issue to you. If you have any questions, you may leave them here.

Please check out our welcome and general setup documentation pages for getting started with setting up your local environment.

@openverse-bot openverse-bot moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Sep 11, 2024
@krysal krysal linked a pull request Nov 6, 2024 that will close this issue
Open
8 tasks
@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Dec 10, 2024
@rajatsingh23
Copy link

I this issue still open. I'm interested to solve this issue

@dhruvkb
Copy link
Member

dhruvkb commented Dec 21, 2024

@rajatsingh23 a PR #5136 has already been opened to fix this issue. I would suggest picking one of the other open issues to work on. For example, you can look at the list of good first issues for an introduction to the code base or the list of help-wanted issues if you feel a bit more comfortable and would like something challenging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
💻 aspect: code Concerns the software code in the repository 🧰 goal: internal improvement Improvement that benefits maintainers, not users good first issue New-contributor friendly help wanted Open to participation from the community 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: catalog Related to the catalog and Airflow DAGs 🔧 tech: airflow Involves Apache Airflow
Projects
Openverse Backlog
Status: 🏗 In Progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Updated notify_slack method ashiramin/openverse
4 participants
@AetherUnbound @dhruvkb @triii10 @rajatsingh23