Skip to content

Conversation

@bbirdsong2
Copy link
Contributor

@bbirdsong2 bbirdsong2 commented Apr 29, 2024

Description

This PR adds a new PPA Dev DAP Collector DAG to run the new docker-etl job here: mozilla/docker-etl#189. This DAG will be used to test out the ads use case in the DAP infrastructure.

Related Tickets & Documents

@bbirdsong2 bbirdsong2 force-pushed the add-dap-collector-ppa-dev-dag branch from 38e85a9 to 9bcd194 Compare April 29, 2024 20:35
@bbirdsong2 bbirdsong2 changed the title Add DAP Collector dag for PPA Dev feat(dags): Add DAP Collector dag for PPA Dev Apr 29, 2024
default_args=default_args,
doc_md=DOCS,
schedule_interval="*/15 * * * *",
tags=tags,
Copy link
Contributor

Choose a reason for hiding this comment

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

You might want to explicitly set catchup=False so it doesn't trigger all runs since the start_date/last run if it's not necessary. False might be the default value but I can't remember

Suggested change
tags=tags,
tags=tags,
catchup=False,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I can add that.

Comment on lines 38 to 39
"retries": 1,
"retry_delay": timedelta(minutes=15),
Copy link
Contributor

Choose a reason for hiding this comment

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

Retry delay is the same as schedule interval so you'll have two running concurrently if it fails. Would it make sense to wait for the next run instead of retrying? Or is the continuity important?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense. I copied this from another job and did not evaluate the retry logic.

If a job fails, I do not actually think a retry or subsequent run would work, because the job depends on the {{ ts }} variable of the dag run, and only goes back every 15 minutes.

I think what I can do is remove the retry and run the script manually if a job fails.

@quiiver quiiver enabled auto-merge (squash) May 3, 2024 19:53
@quiiver quiiver merged commit a6091ac into mozilla:main May 3, 2024
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.

5 participants