crowdin / crowdin-cli

A command-line client for the Crowdin API
https://crowdin.github.io/crowdin-cli
MIT License
250 stars 94 forks source link

Do not return Exit Code Null if all files failed to download via CLI #822

Closed Andrulko closed 4 months ago

Andrulko commented 4 months ago

CLI is a part of GitHub Action in my case, and it may happen that my configuration file (crowdin.yml) is outdated, so nothing will be fetched.

CLI returns Exit Code Null in this case, and GitHub Action is marked as successful: image

Preferred solution It should return an Exit Code that will help to mark GHA as Failed, any non zero exit code works

iamstarkov commented 4 months ago

it should return non zero exit code in case it failed for any reason: failed to upload sources, failed to download translation, failed to build project, failed to fetch project info, any network issues, any credentials issues, anything else really as well

iamstarkov commented 4 months ago

https://docs.github.com/en/actions/creating-actions/setting-exit-codes-for-actions#setting-a-failure-exit-code-in-a-docker-container-action

if <condition> ; then
  echo "Game over!"
  exit 1
fi

Essentially Java crowdin cli needs to fail with non zero exit code which I assume will fail the docker container run which in turn will fail the GitHub action and the step it’s used in.

andrii-bodnar commented 4 months ago

It fails in case of such errors. The current case with the "Couldn't find any file to download" was an exception.

iamstarkov commented 4 months ago

@andrii-bodnar if cli fails, then the issue is in crowdin/github-action because it doesn't fail

andrii-bodnar commented 4 months ago

Currently, the problem is only with the Couldn't find any file to download case, the CLI should return the exit code 1 but returns 0.

iamstarkov commented 4 months ago

I disagree, for example we have failure to download translations due to another build in progress which still results in successful green step

yevheniyJ commented 4 months ago

I think here it does make sense to return error exit code. CLI tries to download as much as it can, some files could be omitted due to some configuration mismatch and that’s all fine if we still were able to download at least something. Here it did nothing. Throwing error exit code here will help to quickly react and fix potential issues with configuration.

iamstarkov commented 4 months ago

thank you

ben-wharton commented 4 months ago

As we mentioned here, we actually have a workflow where we expect no files to be downloaded fairly often, and it should not throw an error code in this case.

We use skip_untranslated_files, which means it's common for nothing to be downloaded if new sources have just been pushed and not translated yet. We don't want our workflow to fail in this case, it makes our builds appear to fail.