You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jeremy Daggett <no...@github.com> on 2014/12/02 00:00:54 UTC
[jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
This PR refactors OpenStack/Rackspace APIs/Providers to use AutoService for service loader metadata generation.
- Updated poms with AutoService dependency
- Cleaned up osgi imports in several poms
- Added `@AutoService(ApiMetadata.class)` annotations to APIs
- Added `@AutoService(ProviderMetadata.class)` annotations to Providers
See [jclouds-labs-openstack PR 172](https://github.com/jclouds/jclouds-labs-openstack/pull/172)
You can merge this Pull Request by running:
git pull https://github.com/rackspace/jclouds refactor/autoservice
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/621
-- Commit Summary --
* Use AutoService to generate service loader metadata
-- File Changes --
M apis/openstack-cinder/pom.xml (11)
M apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/CinderApiMetadata.java (5)
D apis/openstack-cinder/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (18)
M apis/openstack-keystone/pom.xml (13)
M apis/openstack-keystone/src/main/java/org/jclouds/openstack/keystone/v2_0/KeystoneApiMetadata.java (5)
D apis/openstack-keystone/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (1)
M apis/openstack-nova-ec2/pom.xml (12)
M apis/openstack-nova-ec2/src/main/java/org/jclouds/openstack/nova/ec2/NovaEC2ApiMetadata.java (3)
D apis/openstack-nova-ec2/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (1)
M apis/openstack-nova/pom.xml (12)
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/NovaApiMetadata.java (5)
D apis/openstack-nova/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (1)
M apis/openstack-swift/pom.xml (5)
M apis/openstack-swift/src/main/java/org/jclouds/openstack/swift/v1/SwiftApiMetadata.java (3)
D apis/openstack-swift/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (18)
M apis/openstack-trove/pom.xml (13)
M apis/openstack-trove/src/main/java/org/jclouds/openstack/trove/v1/TroveApiMetadata.java (5)
D apis/openstack-trove/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (18)
M apis/rackspace-clouddns/pom.xml (11)
M apis/rackspace-clouddns/src/main/java/org/jclouds/rackspace/clouddns/v1/CloudDNSApiMetadata.java (3)
D apis/rackspace-clouddns/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (1)
M apis/rackspace-cloudidentity/pom.xml (11)
M apis/rackspace-cloudidentity/src/main/java/org/jclouds/rackspace/cloudidentity/v2_0/CloudIdentityApiMetadata.java (2)
D apis/rackspace-cloudidentity/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (1)
M apis/rackspace-cloudloadbalancers/pom.xml (6)
M apis/rackspace-cloudloadbalancers/src/main/java/org/jclouds/rackspace/cloudloadbalancers/v1/CloudLoadBalancersApiMetadata.java (7)
D apis/rackspace-cloudloadbalancers/src/main/resources/META-INF/services/org.jclouds.apis.ApiMetadata (1)
M providers/rackspace-cloudblockstorage-uk/pom.xml (12)
M providers/rackspace-cloudblockstorage-uk/src/main/java/org/jclouds/rackspace/cloudblockstorage/uk/CloudBlockStorageUKProviderMetadata.java (4)
D providers/rackspace-cloudblockstorage-uk/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-cloudblockstorage-us/pom.xml (12)
M providers/rackspace-cloudblockstorage-us/src/main/java/org/jclouds/rackspace/cloudblockstorage/us/CloudBlockStorageUSProviderMetadata.java (4)
D providers/rackspace-cloudblockstorage-us/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-clouddatabases-uk/pom.xml (12)
M providers/rackspace-clouddatabases-uk/src/main/java/org/jclouds/rackspace/clouddatabases/uk/CloudDatabasesUKProviderMetadata.java (4)
D providers/rackspace-clouddatabases-uk/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-clouddatabases-us/pom.xml (12)
M providers/rackspace-clouddatabases-us/src/main/java/org/jclouds/rackspace/clouddatabases/us/CloudDatabasesUSProviderMetadata.java (4)
D providers/rackspace-clouddatabases-us/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-clouddns-uk/pom.xml (12)
M providers/rackspace-clouddns-uk/src/main/java/org/jclouds/rackspace/clouddns/uk/CloudDNSUKProviderMetadata.java (5)
D providers/rackspace-clouddns-uk/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-clouddns-us/pom.xml (12)
M providers/rackspace-clouddns-us/src/main/java/org/jclouds/rackspace/clouddns/us/CloudDNSUSProviderMetadata.java (5)
D providers/rackspace-clouddns-us/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-cloudloadbalancers-uk/pom.xml (5)
M providers/rackspace-cloudloadbalancers-uk/src/main/java/org/jclouds/rackspace/cloudloadbalancers/uk/CloudLoadBalancersUKProviderMetadata.java (5)
D providers/rackspace-cloudloadbalancers-uk/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-cloudloadbalancers-us/pom.xml (5)
M providers/rackspace-cloudloadbalancers-us/src/main/java/org/jclouds/rackspace/cloudloadbalancers/us/CloudLoadBalancersUSProviderMetadata.java (5)
D providers/rackspace-cloudloadbalancers-us/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-cloudservers-uk/pom.xml (12)
M providers/rackspace-cloudservers-uk/src/main/java/org/jclouds/rackspace/cloudservers/uk/CloudServersUKProviderMetadata.java (4)
D providers/rackspace-cloudservers-uk/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
M providers/rackspace-cloudservers-us/pom.xml (12)
M providers/rackspace-cloudservers-us/src/main/java/org/jclouds/rackspace/cloudservers/us/CloudServersUSProviderMetadata.java (4)
D providers/rackspace-cloudservers-us/src/main/resources/META-INF/services/org.jclouds.providers.ProviderMetadata (1)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/621.patch
https://github.com/jclouds/jclouds/pull/621.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
@nacx Last of the cleanup here, thx.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-66326578
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1984](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1984/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65529757
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @jdaggett! Code changes themselves look good to me. A couple of questions
* are we able to verify in any way that the new OSGi imports are correct?
* `provided` for the `auto-service` dep to ensure it's not transitively inherited, or..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65191609
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Ignasi Barrera <no...@github.com>.
>Also, the cloudfiles API and providers were not included because they will be removed before the next release
Could you change them too? That will not cause any overhead when it comes to removing them, but until that happens we'd better have all the existing providers aligned.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65779520
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
@nacx @demobox +1 The intent of this specific PR was the Rackspace/OpenStack code and that was my mindset at that particular time. That said, I have most of the other APIs/Providers converted already, so l was planning to update this PR with those changes.
Since the `auto-service` dependency is just an annotation processor there should be no runtime implications. IIRC, I previously asked if we should pull the dependency up into `jclouds-project/pom.xml`, and I still think its the right thing to do. WDYT?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65314265
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Ignasi Barrera <no...@github.com>.
Oh, sorry, I misunderstood. I think it is OK as-is now, in the dependencyManagement section. We don't want to add dependencies for projects that don't need them. I agree that it only is used at runtime, but at some gras it may slow down the builds.
I'd rather keep it as-is in the pom.xml files (which is what I replied IIRC), just as we do with the jclouds-core dependency, for example.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65315442
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Andrew Phillips <no...@github.com>.
> Should we make it optional across the board?
If that's what the example in the docs say, I'd say go with that..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65291071
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
@nacx 7b596a9 should address your comments. My apologies for any inconvenience.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-66371066
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Ignasi Barrera <no...@github.com>.
There are still services files there... Please, could you make sure you apply the change **everywhere** (it's only about running one command!), even if it is scheduled to be removed, as I commented in my previous comment?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-66354209
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
Pushed to **master** d3c1e2e and **1.8.x** bc48aad
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-69630178
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
@demobox The simplified OSGi import was pointed out to me in a previous PR, so I have been applying it as I go. @ccustine Can you comment on the `import`?
Yes, the `provided` scope is similar to `compile` for the transitive deps. The library is only used for the creation of the metadata at compile time, so I don't believe there is a reason to include it. I also just noticed that the `auto/service` dependency [download page](https://github.com/google/auto/tree/master/service#download) indicates it as `optional`. Should we make it `optional` across the board?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65272510
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Zack Shoylev <no...@github.com>.
+1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65733975
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Andrew Phillips <no...@github.com>.
> We want to make sure that we fix things in jclouds instead of fixing things only here and there.
Excellent point, @nacx...+1 to that.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65301893
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1989](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1989/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65733373
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
@nacx @demobox The remainder of the APIs and Providers have been updated. Let's get this in!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65486503
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Zack Shoylev <no...@github.com>.
Is the dependency still being switched to optional as mentioned above?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65709424
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
Closed #621.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#event-217609105
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @jdaggett!
I think there are still remaining `META-INF/services` files. Could you check (and fix if needed) with the command:
```bash
find . -name 'META-INF' | grep -v target
```
Also, don't squash the commits until it is ready to merge. This helps a lot reviewing incremental changes, specially for big PRs like this one.
And a final note. With all those OSGi import changes, we must make sure we don't break jclouds-karaf. Could you please make sure it builds with the changes in this branch and there are no test failures?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65498216
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Ignasi Barrera <no...@github.com>.
>I previously asked if we should pull the dependency up into jclouds-project/pom.xml, and I still think its the right thing to do. WDYT?
+1. That will also help getting this applied to all providers.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65315025
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Jeremy Daggett <no...@github.com>.
@nacx I updated all other references I could find in the repo, some of which were not totally obvious at first, so thanks for pointing that out! Also, the `cloudfiles` API and providers were not included because they will be removed before the next release. The jclouds-karaf project built with no errors! :+1:
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65512813
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1976](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1976/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65342154
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @jdaggett!
IMO this is only a partial work. If you're doing a change in the jclouds repo, why not addressing the change in all providers, not just the OpenStack ones?
Could you please add the rest of the providers to this PR? We want to make sure that *we fix things in jclouds* instead of *fixing things only here and there*.
I've also seen that you applied this change to the labs-openstack repo. Great! Given that it seems that you're taking ownership of this :) mind changing the remaining repositories too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65300538
Re: [jclouds] Use AutoService for creation of Service Loader Metadata
(#621)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1970](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1970/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/621#issuecomment-65161299