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

Recurring schedule hides configuration errors #358

Open
majkelcc opened this issue Sep 24, 2024 · 10 comments
Open

Recurring schedule hides configuration errors #358

majkelcc opened this issue Sep 24, 2024 · 10 comments

Comments

@majkelcc
Copy link

When configuring recurring jobs I've noticed that SolidQueue is hiding configuration mistakes that make it difficult to catch potential errors:

  1. Jobs that don't pass SolidQueue::RecurringTask's validation are silently filtered out without any error
  2. Setting SOLID_QUEUE_RECURRING_SCHEDULE to a path that doesn't exist makes SolidQueue silently fall back to an empty schedule

Expected behaviour: above errors should trigger an exception.

@majkelcc
Copy link
Author

I also noticed a very odd behaviour:

  • when I pass an invalid path via SOLID_QUEUE_RECURRING_SCHEDULE then SolidQueue defaults to the default config/recurring.yml configuration file
  • when I pass an invalid path via --recurring_schedule_file then SolidQueue is defaulting to an empty schedule (not reading the default config/recurring.yml)

@rosa
Copy link
Member

rosa commented Sep 24, 2024

when I pass an invalid path via SOLID_QUEUE_RECURRING_SCHEDULE then SolidQueue defaults to the default config/recurring.yml configuration file

Hmm... I'm not seeing this. When I pass an invalid path via SOLID_QUEUE_RECURRING_SCHEDULE, the default path in config/recurrring.yml is not used either. The code path for both options (environment variable or CLI option) is the same, so it's strange the behaviour would be different.

@majkelcc
Copy link
Author

I thought I'm just tired, but I can still replicate this behaviour - I'm testing it by adding a new task to config/recurrring.yml - it gets added every time when I'm adding SOLID_QUEUE_RECURRING_SCHEDULE=non_existing_path.yml, doesn't when using --recurring_schedule_file=non_existing_path.yml.

Also this will no longer be an issue when passing an invalid path in any of these methods will result in an error.

@rosa
Copy link
Member

rosa commented Sep 24, 2024

Could you copy the full command you're using to run Solid Queue in each case?

@majkelcc
Copy link
Author

Used commands:

  • SOLID_QUEUE_RECURRING_SCHEDULE=non_existing_path.yml bin/jobs
  • bin/jobs --recurring_schedule_file=non_existing_path.yml

Now I made a syntax error in the default config/recurring.yml and discovered that when using the env variable, SolidQueue is always reading that default configuration file, even when the env variable is pointing to a valid alternative configuration file. When using --recurring_schedule_file the default configuration file is not read and the syntax error there is not producing any errors.

@rosa
Copy link
Member

rosa commented Sep 24, 2024

What version are you running? What you describe sounds a lot like what was fixed in #345 🤔 This would have been included in 1.0.0.beta.

@majkelcc
Copy link
Author

I'm on 0.9.0 😅

Indeed the env variable seems to be simply ignored by cli, I got confused by the combination of these few issues.

@rosa
Copy link
Member

rosa commented Sep 24, 2024

Ahhhh! So sorry about that! It should be fixed now 😅

About the issue with the configuration, yes, it's on my radar; I've got it in my plans to raise or print some warnings about invalid configuration.

@majkelcc
Copy link
Author

About the issue with the configuration, yes, it's on my radar; I've got it in my plans to raise or print some warnings about invalid configuration.

Raise please 😅

Also reading the code I could see that recurring.yml supports nesting tasks under rails-env keys (production, staging, etc.) - knowing this would save me some time. It might be quite a common thing to have recurring tasks tied to specific environments, might make sense to add this example in the docs ✌️

@rosa
Copy link
Member

rosa commented Sep 25, 2024

Also reading the code I could see that recurring.yml supports nesting tasks under rails-env keys (production, staging, etc.) - knowing this would save me some time.

Hmm... I think this is standard for all Rails configurations 🤔 But yes, I'll enhance the example with this.

rosa added a commit that referenced this issue Sep 25, 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

No branches or pull requests

2 participants