You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2017/10/10 08:00:42 UTC
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/859
Delete old deprecated code
In this PR, I've only deleted things deprecated in 0.9.0 or before. I haven't deleted all of it either - I didn't delete the deprecated code that was still in use in non-trivial ways.
We could do with another look through the code for more things to delete, and could also look at deleting things deprecated since 0.10.0 (released 21 Dec 2016 - http://search.maven.org/#search%7Cgav%7C1%7Cg%3A%22org.apache.brooklyn%22%20AND%20a%3A%22brooklyn-core%22).
I've tried to be conservative, to not delete anything that might affect historic persisted state. For example, the only classes I've deleted are ones that I'd really hope are not referenced in anyone's really old persisted state!
This will require some other minor changes in brooklyn-library etc (to remove use of deprecated code), and will likely affect some downstream projects (e.g. https://github.com/brooklyncentral/advanced-networking).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server delete-old-deprecated-code
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/859.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 #859
----
commit 6b25fd86e959c5bbd004111148bcbe6e9bab1874
Author: Aled Sage <al...@gmail.com>
Date: 2017-10-09T09:59:10Z
Deletes old deprecated code
commit 8594e655161ddd308080085f91d4a5f316e1bf6e
Author: Aled Sage <al...@gmail.com>
Date: 2017-10-09T22:46:52Z
Regenerate osgi bundles, fixing deprecation usage
----
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
Apache jenkins failed with:
```
Build timed out (after 60 minutes). Marking the build as aborted.
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test (default-test) on project brooklyn-core: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test failed: The forked VM terminated without properly saying goodbye. VM crash or System.exit called?
```
The last thing in the console before that was the log below (looks like it hung when trying to execute `YamlRollingTimeWindowMeanEnricherTest` - hard to tell if that was the test's fault or the environment's fault):
```
2017-10-11 08:26:43,528 INFO TESTNG INVOKING CONFIGURATION: "Surefire test" - @BeforeMethod org.apache.brooklyn.enricher.stock.YamlRollingTimeWindowMeanEnricherTest.setUp()
2017-10-11 08:26:43,528 INFO Added external config supplier named 'brooklyn-demo-sample': org.apache.brooklyn.core.config.external.InPlaceExternalConfigSupplier@5122387
Build timed out (after 60 minutes). Marking the build as aborted.
```
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
Test failure looked unrelated, and I couldn't reproduce locally (or see why this would fail!):
```
java.lang.AssertionError: failed succeeds-eventually, 21 attempts, 3001ms elapsed: AssertionError: entity=TestEntityImpl{id=rk7nfi2oqq}; attribute=Sensor: service.state (org.apache.brooklyn.core.entity.lifecycle.Lifecycle) expected [stopped] but found [stopping]
at org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogicTest.assertAttributeEqualsEventually(ServiceStateLogicTest.java:338)
at org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogicTest.testStopsNicelyToo(ServiceStateLogicTest.java:143)
Caused by: java.lang.AssertionError: entity=TestEntityImpl{id=rk7nfi2oqq}; attribute=Sensor: service.state (org.apache.brooklyn.core.entity.lifecycle.Lifecycle) expected [stopped] but found [stopping]
at org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogicTest.assertAttributeEqualsEventually(ServiceStateLogicTest.java:338)
at org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogicTest.testStopsNicelyToo(ServiceStateLogicTest.java:143)
```
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r145066673
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/EntityPredicates.java ---
@@ -488,11 +482,6 @@ public String toString() {
}
}
- /** @deprecated since 0.7.0 use {@link #isManaged()} */ @Deprecated
- public static <T> Predicate<Entity> managed() {
- return isManaged();
- }
-
/** @deprecated since 0.7.0 use {@link #isManaged()}, introduced to allow deserialization of anonymous inner class */
@SuppressWarnings("unused") @Deprecated
private static <T> Predicate<Entity> managedOld() {
--- End diff --
As above: playing it safe in case someone has extremely old persisted state (until we have a nicer way to walk users though detecting/fixing such problems).
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
retest this please
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
@aledsage Build failure seems related:
```
[ERROR] /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/location/ssh/SshMachineLocation.java:[140,7] error: SshMachineLocation is not abstract and does not override abstract method <T>getConfig(HasConfigKey<T>) in Location
where T is a type-variable:
T extends Object declared in method <T>getConfig(HasConfigKey<T>)
[ERROR] /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/location/multi/MultiLocation.java:[56,7] error: MultiLocation is not abstract and does not override abstract method <T>getConfig(HasConfigKey<T>) in Location
where T is a type-variable:
T extends Object declared in method <T>getConfig(HasConfigKey<T>)
[ERROR] /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java:[75,7] error: FixedListMachineProvisioningLocation is not abstract and does not override abstract method <T>getConfig(HasConfigKey<T>) in Location
where T is a type-variable:
T extends Object declared in method <T>getConfig(HasConfigKey<T>)
[ERROR] /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostMachineProvisioningLocation.java:[72,7] error: LocalhostMachineProvisioningLocation is not abstract and does not override abstract method <T>getConfig(HasConfigKey<T>) in Location
where T is a type-variable:
T extends Object declared in method <T>getConfig(HasConfigKey<T>)
[ERROR] /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/core/location/access/PortForwardManagerClient.java:[39,7] error: PortForwardManagerClient is not abstract and does not override abstract method <T>getConfig(HasConfigKey<T>) in Location
where T is a type-variable:
T extends Object declared in method <T>getConfig(HasConfigKey<T>)
[ERROR] /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/core/location/AggregatingMachineProvisioningLocation.java:[42,7] error: AggregatingMachineProvisioningLocation is not abstract and does not override abstract method <T>getConfig(HasConfigKey<T>) in Location
where T is a type-variable:
T extends Object declared in method <T>getConfig(HasConfigKey<T>)
[ERROR] /home/jenkins/jenkins-slave/workspace/brooklyn-server-pull-requests/core/src/main/java/org/apache/brooklyn/core/location/access/PortForwardManagerImpl.java:[78,7] error: PortForwardManagerImpl is not abstract and does not override abstract method <T>getConfig(HasConfigKey<T>) in Location
where T is a type-variable:
T extends Object declared in method <T>getConfig(HasConfigKey<T>)
```
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r144662800
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/EntityPredicates.java ---
@@ -454,12 +454,6 @@ public String toString() {
}
}
- /** @deprecated since 0.7.0 use {@link #locationsIncludes(Location)} */
- @Deprecated
- public static <T> Predicate<Entity> withLocation(final Location location) {
- return locationsIncludes(location);
- }
-
/** @deprecated since 0.7.0 use {@link #locationsIncludes(Location)}, introduced to allow deserialization of anonymous inner class */
@SuppressWarnings("unused") @Deprecated
private static <T> Predicate<Entity> withLocationOld(final Location location) {
--- End diff --
Can we remove this one? Don't think it is use for rebind
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
Thanks @tbouron - I've added back the location.getConfig method that I accidentally deleted. I squashed that commit, so we won't have a weird deletion in our history.
I've also rebased against master.
Once jenkins builds, is this good to merge @tbouron?
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r144589344
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java ---
@@ -112,8 +112,8 @@ public static BrooklynClassLoadingContext newClassLoadingContext(@Nullable Manag
return newClassLoadingContext(mgmt, catalogItemId, libraries, JavaBrooklynClassLoadingContext.create(mgmt));
}
- @Deprecated /** @deprecated since 0.9.0; becoming private because we should now always have a registered type callers can pass instead of the catalog item id */
- public static BrooklynClassLoadingContext newClassLoadingContext(@Nullable ManagementContext mgmt, String catalogItemId, Collection<? extends OsgiBundleWithUrl> libraries, BrooklynClassLoadingContext loader) {
+ @Deprecated /** @deprecated since 0.9.0; we should now always have a registered type callers can pass instead of the catalog item id */
+ private static BrooklynClassLoadingContext newClassLoadingContext(@Nullable ManagementContext mgmt, String catalogItemId, Collection<? extends OsgiBundleWithUrl> libraries, BrooklynClassLoadingContext loader) {
--- End diff --
Keeping for persistence reason I suppose?
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r145064284
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/EntityPredicates.java ---
@@ -454,12 +454,6 @@ public String toString() {
}
}
- /** @deprecated since 0.7.0 use {@link #locationsIncludes(Location)} */
- @Deprecated
- public static <T> Predicate<Entity> withLocation(final Location location) {
- return locationsIncludes(location);
- }
-
/** @deprecated since 0.7.0 use {@link #locationsIncludes(Location)}, introduced to allow deserialization of anonymous inner class */
@SuppressWarnings("unused") @Deprecated
private static <T> Predicate<Entity> withLocationOld(final Location location) {
--- End diff --
If someone has really old historic persisted state then they might reference this anonymous class!
At some point (when we have a way for the user to fix their persisted state easily?!) we'll delete this stuff.
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r144662878
--- Diff: core/src/main/java/org/apache/brooklyn/core/entity/EntityPredicates.java ---
@@ -488,11 +482,6 @@ public String toString() {
}
}
- /** @deprecated since 0.7.0 use {@link #isManaged()} */ @Deprecated
- public static <T> Predicate<Entity> managed() {
- return isManaged();
- }
-
/** @deprecated since 0.7.0 use {@link #isManaged()}, introduced to allow deserialization of anonymous inner class */
@SuppressWarnings("unused") @Deprecated
private static <T> Predicate<Entity> managedOld() {
--- End diff --
Can we remove this one? Don't think it is use for rebind
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
retest this please
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r144588755
--- Diff: api/src/main/java/org/apache/brooklyn/api/location/Location.java ---
@@ -89,29 +86,6 @@
boolean containsLocation(Location potentialDescendent);
/**
- * Convenience method for {@code config().get(key)}
- *
- * @see {@link #getConfig(ConfigKey)}
- */
- <T> T getConfig(HasConfigKey<T> key);
--- End diff --
Ooops, went a bit too far on this one, isn't it?
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r144663987
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/osgi/BundleMaker.java ---
@@ -110,8 +110,6 @@ public File createJarFromClasspathDir(String path) {
addUrlItemRecursively(zout, path, path, Predicates.alwaysTrue());
}
- Streams.closeQuietly(zout);
--- End diff --
Should this be removed?
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r145067746
--- Diff: api/src/main/java/org/apache/brooklyn/api/location/Location.java ---
@@ -89,29 +86,6 @@
boolean containsLocation(Location potentialDescendent);
/**
- * Convenience method for {@code config().get(key)}
- *
- * @see {@link #getConfig(ConfigKey)}
- */
- <T> T getConfig(HasConfigKey<T> key);
--- End diff --
Great spot - thanks for checking thoroughly!
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
retest this please
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
retest this please
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
Yup @aledsage, LGTM.
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
I ran this test many times (`@Test(invocationCount=1000)`) and found the deadlock reported in https://issues.apache.org/jira/browse/BROOKLYN-544 (which is not related to this PR).
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r145067457
--- Diff: core/src/main/java/org/apache/brooklyn/util/core/osgi/BundleMaker.java ---
@@ -110,8 +110,6 @@ public File createJarFromClasspathDir(String path) {
addUrlItemRecursively(zout, path, path, Predicates.alwaysTrue());
}
- Streams.closeQuietly(zout);
--- End diff --
Yes - it's done in the finally block as well, so this line is redundant (admittedly not to do with deprecated code, but is general code cleanup).
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/859#discussion_r145063507
--- Diff: core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java ---
@@ -112,8 +112,8 @@ public static BrooklynClassLoadingContext newClassLoadingContext(@Nullable Manag
return newClassLoadingContext(mgmt, catalogItemId, libraries, JavaBrooklynClassLoadingContext.create(mgmt));
}
- @Deprecated /** @deprecated since 0.9.0; becoming private because we should now always have a registered type callers can pass instead of the catalog item id */
- public static BrooklynClassLoadingContext newClassLoadingContext(@Nullable ManagementContext mgmt, String catalogItemId, Collection<? extends OsgiBundleWithUrl> libraries, BrooklynClassLoadingContext loader) {
+ @Deprecated /** @deprecated since 0.9.0; we should now always have a registered type callers can pass instead of the catalog item id */
+ private static BrooklynClassLoadingContext newClassLoadingContext(@Nullable ManagementContext mgmt, String catalogItemId, Collection<? extends OsgiBundleWithUrl> libraries, BrooklynClassLoadingContext loader) {
--- End diff --
The other three public methods all delegate to this; we just want this one to be private.
I expect there's scope to re-write the methods to delete this, but I've not looked closely (hence leaving it as deprecated).
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
Failed again, this time when running `org.apache.brooklyn.entity.machine.MachineEntityJcloudsRebindTest.testRebind`:
```
java.lang.AssertionError: failed succeeds-eventually, 75 attempts, 30002ms elapsed: AssertionError: Commands (/etc/os-release) not contain in ...
at org.apache.brooklyn.test.Asserts.succeedsEventually(Asserts.java:1009)
at org.apache.brooklyn.test.Asserts.succeedsEventually(Asserts.java:892)
at org.apache.brooklyn.test.Asserts.succeedsEventually(Asserts.java:885)
at org.apache.brooklyn.entity.machine.MachineEntityJcloudsRebindTest.assertRecordedSshCmdContainsEventually(MachineEntityJcloudsRebindTest.java:117)
at org.apache.brooklyn.entity.machine.MachineEntityJcloudsRebindTest.testRebind(MachineEntityJcloudsRebindTest.java:103)
Caused by: java.lang.AssertionError: Commands (/etc/os-release) not contain in [ExecCmd{summaryForLogging=, commands=[uptime], env={}, constructorProps={...
at org.apache.brooklyn.entity.machine.MachineEntityJcloudsRebindTest$3.run(MachineEntityJcloudsRebindTest.java:125)
at org.apache.brooklyn.test.Asserts$RunnableAdapter.call(Asserts.java:1363)
at org.apache.brooklyn.test.Asserts.succeedsEventually(Asserts.java:970)
... 33 more
```
Higher in the test log, it says:
```
2017-10-17 15:01:36,289 WARN Could not resolve on-box directory for MachineEntityImpl{id=hodqzzl5nj} at SshMachineLocation[173.194.32.1:jenkins@173.194.32.1/173.194.32.1:22(id=ys5w4aqplj)]; using /home/jenkins/brooklyn-managed-processes, though this may not be accurate at the target (and may fail shortly)
2017-10-17 15:01:36,290 WARN Configuration being made to SshMachineLocation[173.194.32.1:jenkins@173.194.32.1/173.194.32.1:22(id=ys5w4aqplj)] after deployment; may not be supported in future versions
2017-10-17 15:01:36,517 INFO Adding machine metrics feed
2017-10-17 15:01:36,549 WARN unable to compute MachineEntityImpl{id=hodqzzl5nj}->Sensor: machine.memory.total (java.lang.Long); on val=org.apache.brooklyn.feed.ssh.SshPollValue@1bc671b9
java.lang.IndexOutOfBoundsException: Index: 1, Size: 0
at java.util.ArrayList.rangeCheck(ArrayList.java:653) ~[na:1.8.0_144]
2017-10-17 15:01:36,549 WARN unable to compute MachineEntityImpl{id=hodqzzl5nj}->Sensor: machine.memory.used (java.lang.Long); on val=org.apache.brooklyn.feed.ssh.SshPollValue@1bc671b9
java.lang.IndexOutOfBoundsException: Index: 2, Size: 0
at java.util.ArrayList.rangeCheck(ArrayList.java:653) ~[na:1.8.0_144]
2017-10-17 15:01:36,550 WARN unable to compute MachineEntityImpl{id=hodqzzl5nj}->Sensor: machine.memory.free (java.lang.Long); on val=org.apache.brooklyn.feed.ssh.SshPollValue@1bc671b9
java.lang.IndexOutOfBoundsException: Index: 3, Size: 0
at java.util.ArrayList.rangeCheck(ArrayList.java:653) ~[na:1.8.0_144]
2017-10-17 15:01:36,550 WARN unable to compute MachineEntityImpl{id=hodqzzl5nj}->Sensor: machine.loadAverage (java.lang.Double); on val=org.apache.brooklyn.feed.ssh.SshPollValue@22c4f515
java.lang.NullPointerException: null
at org.apache.brooklyn.entity.machine.AddMachineMetrics$2.apply(AddMachineMetrics.java:112) ~[classes/:na]
```
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
retest this please
---
[GitHub] brooklyn-server issue #859: Delete old deprecated code
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/859
retest this please
---
[GitHub] brooklyn-server pull request #859: Delete old deprecated code
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/859
---