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

Allow tasks to be loaded from nested task directories #809

Merged
merged 5 commits into from
Apr 19, 2023

Conversation

styrmis
Copy link
Contributor

@styrmis styrmis commented Apr 19, 2023

In shop-server we have tasks which are in sub-folders (e.g. app/tasks/maintenance/deep_understanding). These are available in production, but in development they are not automatically loaded, making them harder to test (TODO: verify this).

Before this PR, only tasks directly under app/tasks will be loaded in development. After this commit, one level of nesting is supported.

Copy link
Member

@etiennebarrie etiennebarrie left a comment

Choose a reason for hiding this comment

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

#752 was already a fix in progress for this, maybe you can combine the two since we were waiting for the tests? That other implementation used a lambda to recursively load all the constants defined in modules found in namespace, which seems more correct.

@styrmis styrmis marked this pull request as ready for review April 19, 2023 14:48
@styrmis
Copy link
Contributor Author

styrmis commented Apr 19, 2023

Thanks @etiennebarrie, I wasn't aware of that one. I've cherry-picked the commit from that PR and have extended the tests to cover two levels of nesting. I've also added a section to the README.

Copy link
Member

@etiennebarrie etiennebarrie left a comment

Choose a reason for hiding this comment

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

Thanks for including examples tasks (I just fixed them to make the succeed when you run them) and a README section!!

@etiennebarrie etiennebarrie merged commit 1dfa042 into main Apr 19, 2023
@etiennebarrie etiennebarrie deleted the allow-nested-tasks branch April 19, 2023 15:37
lawrencewong pushed a commit to lawrencewong/maintenance_tasks that referenced this pull request Apr 29, 2023
Co-authored-by: Masa (Aileron inc) <masa@aileron.cc>
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.

None yet

3 participants