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

Deployment continues even when setup:static-content:deploy contains exception #44

Closed
erikhansen opened this issue Nov 15, 2016 · 8 comments

Comments

@erikhansen
Copy link
Contributor

erikhansen commented Nov 15, 2016

There is a long-standing bug that prevents new themes from being registered when Magento is in production mode. This means that when Capistrano tries to deploy a new theme to the server, an error like [LogicException] Unable to load theme by specified key: 'ClassyLlama/example' will occur.

This Gem is not recognizing this output as an error, so the deployment proceeds even after error is triggered:

23-02-31 new message-dfojq

I would think the https://github.com/davidalger/capistrano-magento2/blob/master/lib/capistrano/magento2.rb#L44..L48 section should be expanded to check for the presence of [.*Exception] in the output. Doing this search will catch the different types of exceptions that can be thrown during asset compilation. Look at the two exceptions in this method for an example: https://github.com/magento/magento2/blob/1ade3b769a937f19682bbbe4e51b6c956147952f/lib/internal/Magento/Framework/View/Design/Theme/FlyweightFactory.php#L53

@hectornguyen
Copy link

I'm interested in a workaround for this issue.

@erikhansen
Copy link
Contributor Author

@hectornguyen I assume you're wanting a workaround for the "Unable to load theme specified by key ******" issue, since you commented on the magento/magento2#2797 thread. I think the solution that most people have adopted (including myself) is to manually insert the new theme into the theme table. See this comment: magento/magento2#2797 (comment)

@hectornguyen
Copy link

@erikhansen Actually, I'm looking for a workaround for this issue. I do apologise for that silly question, because I didn't notice to your commit

"setup:static-content:deploy #{params} | stdbuf -o0 tr -d .; test ${PIPESTATUS[0]} -eq 0",

Big fan of Capistrano although I hate Ruby 👯‍♂️

@davidalger
Copy link
Owner

@erikhansen

Would you check on a few things?

  1. Can you determine which version (i.e. is it 2.1.0 or 2.1.1, etc) which introduced the error codes in the CLI tool?
  2. The work done in Static asset compilation errors are ignored #35 for similar failure to catch error output; this was unfortunately also as a result of the exit code hiding behind the pipe. Are you able to verify that those errors also do in-fact produce an exit code in versions which issue them? If not, that's a core-bug, but I'd like to know as well as be able to not have the crazy string checking in versions which don't require it.

I've closed the corresponding PR in favor of resolving the exit code errors by using a mapping prefix in SSHKit. See commit d5c9810. I even verified that setting it this way is in fact isolated to just this command and won't affect subsequent commands.

If you could test this on the hash failing the deploy and confirm resolution, that would be great. To do that, checkout the issue-44 branch, rake install, then run the deploy without using bundler exec (so it uses what you just installs).

@erikhansen
Copy link
Contributor Author

@davidalger

  1. The error codes were added in 2.1.1
  2. Yes, I'll verify this.

I won't be able to get to this until early next week.

@erikhansen
Copy link
Contributor Author

@davidalger I just tested the issue-44 branch and can confirm that it works as expected.

The work done in #35 for similar failure to catch error output; this was unfortunately also as a result of the exit code hiding behind the pipe. Are you able to verify that those errors also do in-fact produce an exit code in versions which issue them?

^ To avoid doing unnecessary string checking in 2.1.1+, I added a version check and have issued a PR to your issue-44 branch. I tested this check and confirmed that deploying a 2.1.2 site with a "Compilation from source" compilation error resulted in halted deployment, as expected.

@davidalger
Copy link
Owner

@erikhansen Great, thanks for confirming. In the case of a version which issues the error code, the string check is already skipped since the error code results in an exception. I mostly wanted to confirm the version number for documentation and change log sake. Thanks!

I'll get this pushed up shortly.

@davidalger
Copy link
Owner

Resolved in v0.5.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants