Warning added for save only action #1034
Conversation
| // "forwardPorts": [], | ||
| // Use 'postCreateCommand' to run commands after the container is created. | ||
| "postCreateCommand": "npm install && npm run build" | ||
| "postCreateCommand": "npm install" |
There was a problem hiding this comment.
npm run build take a very long time while setting up the codespace, hence removing it from here.
dist/save/index.js
Outdated
| try { | ||
| if (!utils.isCacheFeatureAvailable()) { | ||
| return; | ||
| return 0; |
There was a problem hiding this comment.
We will return 0 when we don't want to log a warning, and -1 when we want to log a warning.
dist/save/index.js
Outdated
| try { | ||
| if (!utils.isCacheFeatureAvailable()) { | ||
| return; | ||
| return 0; |
There was a problem hiding this comment.
0 may be a real cacheId as DB entry starts at 0. Should we use something like -2
There was a problem hiding this comment.
Yes we can. I was thinking, in the future we can also map the negative numbers to some error types to decide how to log/show error to the user.
There was a problem hiding this comment.
Lets do that now itself. We can define an enum on cache return type.
There was a problem hiding this comment.
Actually re-thinking about this. Why do we need to return id here. Can we return error here. Any error and SaveOnly should print warning message. No?
There was a problem hiding this comment.
Does this mean we also need to implement error handling in save.ts?
Description
For save only action, it is important for users to know that the save has failed due to any reason, hence we've added a warning to show the same.
Motivation and Context
Make user experience better by highlighting any warnings/issues in the workflow run
How Has This Been Tested?
Local workflow
Save action - https://github.com/kotewar/test-continue-on-error-for-cache/actions/runs/3742275202
Default cache action - https://github.com/kotewar/test-continue-on-error-for-cache/actions/runs/3742531272/jobs/6353592058#step:13:665
Screenshots (if appropriate):
NA
Types of changes
Checklist: