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

Set ingestion timer trigger to never run in staging #198

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

somesylvie
Copy link
Contributor

Description

Until the mu character ingestion issues are fixed, turn off auto ingestion in staging so that files won't have to be re-uploaded

Issue

CDCgov/trusted-intermediary#1467

Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@@ -30,5 +30,5 @@ module "template" {

environment = "stg"
deployer_id = "f5feabe7-5d37-40ba-94f2-e5c0760b4561" //github app registration in CDC Azure Entra
cron = "0 30 9 * * 1-5" // Every weekday at 9:30 AM
cron = "* * * 30 Feb *" //run every second of February 30th, which never happens and is the equivalent of never running

Choose a reason for hiding this comment

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

Consider using a more explicit method to disable the cron job, such as setting the cron expression to an empty string or using a conditional expression based on the environment. This will make the intent clearer and avoid potential confusion with the use of a non-existent date. [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.

An empty CRON expression is not valid here

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Best practice
Replace the non-standard cron setting with a more conventional method to prevent execution

*Instead of using a non-existent date to prevent the cron job from running,
explicitly set the cron configuration to an empty string or use a more conventional
approach like setting it to '0 0 31 2 ' which is a valid but non-existent date
(February 31st).

operations/environments/stg/main.tf [33]

-cron        = "* * * 30 Feb *"                       //run every second of February 30th, which never happens and is the equivalent of never running
+cron        = ""                                    // Explicitly set to never run
Suggestion importance[1-10]: 7

Why: The suggestion to replace a non-standard cron expression with a more conventional or explicit method enhances clarity and maintainability. Using a non-existent date like "30 Feb" could be confusing, whereas an empty string or a valid non-existent date like "31 Feb" is clearer and adheres to common practices.

7

Copy link

@somesylvie somesylvie merged commit 62e7631 into main Oct 22, 2024
14 checks passed
@somesylvie somesylvie deleted the turn-off-ingestion-task-in-staging branch October 22, 2024 15:51
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.

2 participants