Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

return a nonzero error code when missing an item in the cache #70

Closed
ekimia opened this issue May 22, 2017 · 9 comments
Closed

return a nonzero error code when missing an item in the cache #70

ekimia opened this issue May 22, 2017 · 9 comments
Labels

Comments

@ekimia
Copy link
Contributor

ekimia commented May 22, 2017

Enhancement Suggestion

Main use case: pulling in the cache from CI, seeing there isnt a rome cache, and then moving onto running `carthage update` followed by `rome upload`. 

**Current and suggested behavior**

Currently it outputs the "could not find .frameworkname.version in local cache at: CACHE_DIR but with a 0 exit code.

**Why would the enhancement be useful to most users**

Automatically cache and upload if the cache isn't present

**Rome version:**  0.11.27
**OS and version:** Sierra
@ekimia
Copy link
Contributor Author

ekimia commented May 22, 2017

Would totally be down to submit a PR but wanted to get the feelers out about this change.

@tmspzz
Copy link
Owner

tmspzz commented May 23, 2017

Hi @ekimia, thanks for opining an issue.

If I understand correctly you want to build what is missing in the cache and then upload it.
This use case is already supported as described in https://github.com/blender/Rome#listing but as I just realised the CI workflow is not obvious.

What you want to do is list before building/uploading like so:

# Rome CI workflow

rome download --platform iOS # download missing frameworks (or copy from local cache)
rome list --missing --platform ios | awk '{print $1}' | xargs carthage update --platform ios # list what is missing and update/build if needed
rome list --missing --platform ios | awk '{print $1}' | xargs rome upload --platform ios # upload what is missing

If no frameworks are missing the pipe to awk will fail and the rest of the command will not be executed. This avoids rebuilding all dependencies or uploading frameworks already present in the cache. For the sake of understanding the complete command you can call list with --present to get a feeling for the different steps.

Does this help in supporting your workflow?

The download command could be further enhanced to even skip copying file locally if your Carthage .version files are present and frameworks are at the correct hash.

@tmspzz
Copy link
Owner

tmspzz commented May 23, 2017

I have opened a PR to enhance the documentation on the CI workflow. Feel free to comment: https://github.com/blender/Rome/blob/fix/ci-workflow-readme/README.md#ci-workflow

@ekimia
Copy link
Contributor Author

ekimia commented May 23, 2017

ah this is perfect! Great idea to enhance the documentation.

@tmspzz
Copy link
Owner

tmspzz commented May 23, 2017

@ekimia here is the PR #71, let me know if you have comments.

About exiting with non-zero status from a download command there are the following considerations to make:

  • exit with non-zero when ANY artifact download fails?
    • each artifact download failure could be encoded as a bit pattern and ORed together in a final error code
      • failing framework: 0x02
      • failing dSYM: 0x04
      • failing .versionFile: 0x8
  • exit with non-zero only when framework download fails?
  • exit with non-zero only on runtime errors (current behaviour)
    • bad .aws credentials
    • conflicting parameters
    • unrecoverable network error
    • ...

@netbe
Copy link
Contributor

netbe commented May 24, 2017

I would be up for

exit with non-zero only when framework download fails?

@erichoracek
Copy link

erichoracek commented May 24, 2017

I think this really depends on what you consider rome's job to be. In my opinion:

If you perform rome download and you get a partial hit, this seems like rome did what you asked to the best of its ability. There is no way that rome could have succeeded any more concretely beside there more artifacts being present in the cache, which will never change no matter how many times you run rome download. As for whether or not this is a non-zero exit status, this depends on whether you think rome download means "warm the carthage build folder with cached frameworks" or "download every possible framework, otherwise bail". For me, I interpret it as the former since I see rome as a cache in front of carthage. In my experience with caching APIs, a cache miss is typically not a exception-style failure, just one of two possible successful outcomes (hit/miss).

@tmspzz
Copy link
Owner

tmspzz commented May 24, 2017

I agree with @erichoracek , a miss is just one of the possible outcomes, it's not an exceptional failure that terminates the program execution.

For the time being I'll leave things as they are. If the issue is brought up again I'll consider exiting with non-zero status.

@ekimia please consider closing the issue if the explanations I gave about the CI workflow are enough for you.

@tmspzz
Copy link
Owner

tmspzz commented May 29, 2017

Closing for now.

@tmspzz tmspzz closed this as completed May 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants