-
-
Notifications
You must be signed in to change notification settings - Fork 182
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
WIP, add pre/post copy and pre/post update tasks #1511
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the initiative, @hahuang65! 🙏
Just a design feedback to start with: Having new top-level keywords _{pre,post}_{copy,update}
looses the context that those are actually task definitions. How about extending the _tasks
syntax similar to #326 and #1510?
_tasks:
# implicitly a post-copy task
- touch hello.txt
# explicitly a pre-copy task using Jinja
- "{% if _stage == 'before_copy' %}touch hello.txt{% endif %}"
# uses environment variable `STAGE={before,after}_{copy,update}`
- ./task.sh
# new convenient syntax (#326, #1510)
- command: touch hello.txt
when: "{{ _stage == 'before' }}"
This would be backwards compatible when using the old syntax and add convenience for conditional tasks when using the new syntax.
thanks for the feedback @sisp! Does It also makes it much more difficult to simply port an existing shell script into the Those are just my thoughts. I'm happy to oblige with your design suggestion, especially if it's already been pre-established. |
@hahuang65 Yes, As it turns out, #1510 is already adding the new, more convenient syntax not only for migrations but also for tasks. It might make sense to wait for it to be merged and then build this PR upon it. |
@hahuang65 If you're interested in continuing work on this PR, you may now build upon the implementation of #1510 that we just merged. |
@sisp exciting! Unfortunately, probably for the next 2 to 3 months I will be busy. If someone would like to take over, feel free. Otherwise, I'll re-visit in a few months. |
Hacking around for a potential solution to #240
Would love some feedback on whether we think this is a feasible solution or not.
If so, I need a little guidance on how to write some tests for the update tasks (they seem a bit difficult to test as we need to set up some Git repos), as well as how to test the difference between a pre and a post task.