Convert remaining GH action scripts to env vars #1615
Conversation
WalkthroughThe changes across the three workflows focus on improving the clarity, maintainability, and debugging capabilities of the deployment process. This is achieved through better use of environment variables, enhanced Docker image handling, and improved Azure deployment commands. The changes also include better use of GitHub variables and input values for testnet types. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Files selected for processing (3)
- .github/workflows/manual-deploy-obscuro-gateway-database.yml (6 hunks)
- .github/workflows/manual-deploy-obscuro-gateway.yml (5 hunks)
- .github/workflows/manual-deploy-obscuro-scan-2.yml (3 hunks)
Files skipped from review due to trivial changes (1)
- .github/workflows/manual-deploy-obscuro-gateway-database.yml
Additional comments (Suppressed): 8
.github/workflows/manual-deploy-obscuro-scan-2.yml (4)
22-23: The environment name is set to the
testnet_typeinput. Ensure that the environment names are unique for each testnet type and that they follow the naming conventions for GitHub environments.54-55: The Docker build and push steps now use the
varsobject to access the Docker build tags. Ensure that these variables are correctly set in thevarsobject before these steps are executed.77-77: The Docker images for the frontend and backend deployments are now retrieved from the
varsobject. Ensure that these variables are correctly set in thevarsobject before these steps are executed.81-81: The command-line argument for the backend deployment now uses the
varsobject to access the L2 RPC URL. Ensure that this variable is correctly set in thevarsobject before this step is executed..github/workflows/manual-deploy-obscuro-gateway.yml (4)
19-28: The environment name is now set based on the
testnet_typeinput, and GitHub variables are printed for debugging purposes. This is a good practice for debugging and understanding the state of the environment variables at the time the job ran. However, ensure that no sensitive information (like secrets or passwords) is being printed in the logs.38-43: The redundant code for setting Obscuro Gateway variables has been removed. This improves code maintainability. However, ensure that these variables are being set elsewhere in the code, as they are crucial for the correct functioning of the workflow.
52-71: The Docker image building and pushing, VM deletion, and creation steps have been updated to use the new environment variables. This is a good practice as it makes the code more maintainable and less prone to errors. However, ensure that these new environment variables are being set correctly elsewhere in the code.
120-127: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [97-127]
The command execution on the VM has been updated to use the new environment variables. This is a good practice as it makes the code more maintainable and less prone to errors. However, ensure that these new environment variables are being set correctly elsewhere in the code.
| - name: 'Print GitHub variables' | ||
| # This is a useful record of what the environment variables were at the time the job ran, for debugging and reference | ||
| run: | | ||
| echo "GitHub Variables = ${{ toJSON(vars) }}" |
There was a problem hiding this comment.
The toJSON(vars) function is used to print the GitHub variables. However, vars is not defined in the context of this workflow. If vars is intended to be a collection of environment variables, consider using env instead, which is a predefined object in GitHub Actions that contains the environment variables.
- echo "GitHub Variables = ${{ toJSON(vars) }}"
+ echo "GitHub Variables = ${{ toJSON(env) }}"
Why this change is needed
Finish standardising the GH action scripts to use the same env variables.
What changes were made as part of this PR
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks