Add new workflow #462
Add new workflow #462
Conversation
.github/workflows/ci-cd-dev.yml
Outdated
| components install kubectl && gcloud composer environments run | ||
| $COMPOSER_ENVIRONMENT --location $LOCATION variables import | ||
| -- gcsfuse/actual_mount_path/variables.json | ||
| # deploy-to-dev: |
There was a problem hiding this comment.
Is this commented code copy and pasted to the new cicd? If so do you think it's worthwhile to keep it commented here or just delete it? We can also always look at git commit history for the file
There was a problem hiding this comment.
yep it is part of new workflow. We can 🔪 it for once
There was a problem hiding this comment.
Addressed in bc4e563
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| envName: |
There was a problem hiding this comment.
Just wanted to confirm; does envName get converted to inputs.env_name?
There was a problem hiding this comment.
Uff no. That's my bad. Let me change it
There was a problem hiding this comment.
Addressed in bc4e563
amishas157
force-pushed
the
github-actions-for-release-proces
branch
from
09a3d0e to
7ae39fd
Compare
get past off annotation error fool tests a bit
amishas157
force-pushed
the
github-actions-for-release-proces
branch
from
7ae39fd to
7085f2f
Compare
There was a problem hiding this comment.
@amishas157 changes are looking good. I did have an overall design question. In your design doc, it proposed that deploys to either env (test, prod) should only come from the master branch. This PR, however, allows the developer to deploy to test off of any branch as needed (no review required).
What were your thoughts on the dev workflow?
- Dev commits feature branch
- when ready, selects branch name and deploys to test
- deploy ci executes
Is this correct?
| - name: Pytest | ||
| run: pytest dags/ |
There was a problem hiding this comment.
I don't know how useful it is to keep the tests job if we aren't actually running the Airflow unit tests.
In the current workflow definition, the first time that the Airflow unit tests will execute is on deploy. Is that intended?
There was a problem hiding this comment.
So, my plan is to remove ci-cd.yml completely. However. the tests job is a required workflow to merge PR. Therefore, I kept it such that it passes.
Once the testing for deploy.yml is done, I will remove this file altogether
Yes, that's still correct. I am currently using this PR to test my changes by deploying to dev. Unfortunately, github actions does not allow testing workflow before merging. One the tests look good, my plan is to create followup PR to:
|
|
Sounds good, thanks for clarifying |
PR Checklist
PR Structure
otherwise).
Thoroughness
What
This PR adds a new github workflow that:
This also turns off dev-to-deploy step as it is not really used currently.
Why
Improve the release workflow.
Known limitations
None