Skip to content

Conversation

@geomacy
Copy link
Contributor

@geomacy geomacy commented Jun 6, 2017

Fixes break from #672,
I merged without re-running unit tests :-(

Fixes break from apache#672,
I merged without re-running unit tests :-(
@neykov
Copy link
Member

neykov commented Jun 6, 2017

LGTM, will wait for the tests to pass and merge.

@geomacy
Copy link
Contributor Author

geomacy commented Jun 6, 2017

Hm, build fails with same error - I can't reproduce that however. I did reproduce the original error, then built everything successfully after making the change above. Odd...

@geomacy
Copy link
Contributor Author

geomacy commented Jun 6, 2017

Hm very strange - it looks as if the test hasn't pulled the change, see

https://builds.apache.org/job/brooklyn-server-pull-requests/ws/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java

Copying the relevant lines as the above will be thrown away on next rebuild:

        @SuppressWarnings("deprecation")
        public static BundleInstallationRestResult of(OsgiBundleInstallationResult in, ManagementContext mgmt, BrooklynRestResourceUtils brooklynU, UriInfo ui) {
            BundleInstallationRestResult result = new BundleInstallationRestResult();
            result.message = in.getMessage();
            result.bundle = in.getMetadata().getVersionedName().toString();
            result.code = in.getCode();
            if (in.getCatalogItemsInstalled()!=null) {
                result.types = MutableMap.of();
                for (String id: in.getCatalogItemsInstalled()) {
                    // TODO prefer to use RegisteredType, but we need transformer for those in REST
                    //RegisteredType ci = mgmt.getTypeRegistry().get(id);
                    
                    CatalogItem<?, ?> ci = CatalogUtils.getCatalogItemOptionalVersion(mgmt, id);
                    CatalogItemSummary summary = CatalogTransformer.catalogItemSummary(brooklynU, ci, ui.getBaseUriBuilder());
                    result.types.put(id, summary);
                }
            }
            return result;
        }

Note the line

            result.bundle = in.getMetadata().getVersionedName().toString();

which doesn't include the change linked above.

@geomacy
Copy link
Contributor Author

geomacy commented Jun 6, 2017

retest this please

@neykov
Copy link
Member

neykov commented Jun 6, 2017

retest this please, after deleting the workspace

@neykov
Copy link
Member

neykov commented Jun 6, 2017

Tests pass locally, merging. Will investigate jenkins issue separately.

@asfgit asfgit merged commit c545381 into apache:master Jun 6, 2017
asfgit pushed a commit that referenced this pull request Jun 6, 2017
Check versionedName to avoid NPE.

Fixes break from #672,
I merged without re-running unit tests :-(
@geomacy
Copy link
Contributor Author

geomacy commented Jun 6, 2017

Looks like it is fetching the apache master instead of the branch from my fork, as below. Note c545381152a22df29d16c063fd6cd2fab4ec7b33 is my change but it is fetching from

Checking out Revision 57b9d7270ba87486f311020bf62b37105d4ca0e4 (refs/remotes/origin/master)

rather than from refs/heads/pr/719.

From build log:

GitHub pull request #719 of commit c545381152a22df29d16c063fd6cd2fab4ec7b33, no merge conflicts.
Setting status of c545381152a22df29d16c063fd6cd2fab4ec7b33 to PENDING with url https://builds.apache.org/job/brooklyn-server-pull-requests/2265/ and message: 'Build started sha1 is merged.'
Using context: Jenkins: brooklyn-server-pull-request
[EnvInject] - Loading node environment variables.
Building remotely on H14 (ubuntu xenial) in workspace /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests
 > git rev-parse --is-inside-work-tree # timeout=10
Fetching changes from the remote Git repository
 > git config remote.origin.url git://github.com/apache/brooklyn-server.git # timeout=10
Fetching upstream changes from git://github.com/apache/brooklyn-server.git
 > git --version # timeout=10
 > git fetch --tags --progress git://github.com/apache/brooklyn-server.git +refs/heads/*:refs/remotes/origin/*
 > git rev-parse refs/remotes/origin/master^{commit} # timeout=10
 > git rev-parse refs/remotes/origin/origin/master^{commit} # timeout=10
Checking out Revision 57b9d7270ba87486f311020bf62b37105d4ca0e4 (refs/remotes/origin/master)
 > git config core.sparsecheckout # timeout=10
 > git checkout -f 57b9d7270ba87486f311020bf62b37105d4ca0e4
First time build. Skipping changelog.
Parsing POMs

@geomacy geomacy deleted the check-on-catresource branch June 6, 2017 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants