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?
3 participants
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: