You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2015/04/22 15:05:43 UTC
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
GitHub user ahgittin opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/610
temporary local copy of fix for HVM/paravirtual issue on AWS
fixes https://issues.apache.org/jira/browse/BROOKLYN-140
as noted, when new jclouds lands we should revert this
@duncangrant can you let us know if this fixes your problem?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/incubator-brooklyn fix-aws-hvm-paravirtual-issue-temporarily
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/610.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #610
----
commit 31b29b305ae366edf590912c5dbae57f6dbcc16e
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2015-04-22T12:57:56Z
temporary local copy of fix for HVM/paravirtual issue on AWS
fixes https://issues.apache.org/jira/browse/BROOKLYN-140
as noted, when new jclouds lands we should revert this
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95627256
Good point by @aledsage , agree with him. We already had an issue with jars ordered differently - one of the jars was shadowing the others, but only if it was before them in the classpath.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95736462
For jar ordering, it is deterministic on a given platform+jvm but different platforms seem to order it differently. There was a fix done like this one before that worked fine locally but then failed when Brooklyn was run on (if I recall correctly) ubuntu with OpenJDK.
This relates to the distro, with the classpath set to include `lib/brooklyn/*.jar`. Ordering in IDEs probably behave completely differently again, as might maven command line builds.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95626523
btw - fixed checkstyle (tab from jclouds project). also have confirmation from @duncangrant that this fixes his problem case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/610
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95922664
Any way we can make the patch work transparently for downstream projects as well? Can't see how it could be approached.
Having `lib/deps` would be fine for Brooklyn, but downstream projects will face the same problem again, because `brooklyn-location-jclouds` will be mixed with the other dependencies.
I like the idea of a separate patches project, with each downstream making sure it puts it in its patches folder. This won't be the last time there is need to override a dependency, so why not go all the way and make it robust. It's a one time change so next time patching will be transparent.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95173919
i've confirmed with this PR i can create an instance with `hardwareId: r3.large` which is one of the types which does only hvm.
note that this may introduce some changed default images, with more changed default images landing with the next jclouds (when new generation instance types are preferred). /cc @aledsage
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95853016
@aledsage should we add a `lib/deps/` dir and order the `dist` classpath so that `lib/brooklyn/*.jar` is first? that seems the easiest way to solve issues with the dist. i'd rather not introduce a patch projects. or as most environments seem to work we could just leave it as is and accept that the aws `hvm` error might remain an issue in some cases until we have a new jclouds cut.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-96548170
Downstream patches could be an elegant solution but I can't help feeling we're trying to do too much. Maintaining a patch dir downstream which grabs patches from upstream feels like a lot of work which is best avoided. Also it won't solve any errors which occur if the downstream project is being run from the IDE.
That said I'm not convinced these errors are severe because both maven order and alpha order give us the desired result and those are the majority strategies.
The right way to support a downstream project, would be to take a tagged jclouds cut with this patch. I'd rather invest in that if required than passing patches, I think. (But even that I don't think is urgent.)
However happy to be overruled if @aledsage feels strongly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-97619549
@aledsage bump. could we merge this before someone else hits hvm issues and if a full-blown patches project is needed we do that (or resolve it with one of the other techniques above).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95597176
isn't it going to be loaded either in alphabetical order (release) or maven order (ide) ?
so i think it *is* deterministic. slightly naughty but pragmatic, for a temporary fix. (haven't we used this trick before for overriding jclouds items (gce in particular)? the only times problems i'm aware of problems is when the overriding jar filename comes first alphabetically.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-100298307
merging this; if we want to do a better patching strategy we can shortly
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95577403
Build fails with:
[INFO] There are 1 checkstyle errors.
[ERROR] EC2HardwareBuilder.java[686:1] (whitespace) FileTabCharacter: Line contains a tab character.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: temporary local copy of fix for H...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/610#issuecomment-95577612
This fix is non-deterministic. There is no guarantee of classpath order, for whether the jclouds class or the replacement class will be first on the classpath. It would need to be created as a patch, and added to the `lib/patch/` dir to guarantee classpath ordering.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---