Re: [jclouds-labs] [JCLOUDS-1046] Remove h2-jdbc provider from source tree (#236)
This is the last year that I am eligible for the GSoC program, maybe I can make a proposal. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/236#issuecomment-188115395
Re: [jclouds-labs] [JCLOUDS-1046] Remove h2-jdbc provider from source tree (#236)
When I implemented this api last summer, I had two requirements: 1. The api had to be independent from the actual database engine. 2. The blobs had to be stored in the database. Since blob operations are usually not standard in SQL, the only sane way to meet these requirements was to add and object mapper. The jdbc api ended up being a jpa api. The jdbc api was supposed to solve the testing incompatibilities we were having with the filesystem provider when storing metadata. I believe that adding an object mapper as a dependency makes the problem even worse. In my honest opinion, it would be better to have a small provider tied to a single database engine (like h2 or sqlite) and use directly jdbc to store only the metadata in the database and the binary file in the filesystem, as @andrewgaul has suggested in the past. In conclusion: yes, I'd remove it :+1: --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/236#issuecomment-187268977
Re: [jclouds-labs] Remove DigitalOcean v1 (#219)
If this only happens with hibernate maybe we should disable the hibernate tests and keep the eclipselink tests, at least until we fix it. It is causing too much trouble. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/219#issuecomment-152671575
Re: [jclouds-labs] JCLOUDS-1009 JCLOUDS-1029: Fix tests and try to avoid deadlocks (#217)
That's wierd, it doesn't happen on my OSX (10.10). I just pushed the pom configuration. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/217#issuecomment-151873880
Re: [jclouds-labs] JCLOUDS-1009 JCLOUDS-1029: Fix tests and try to avoid deadlocks (#217)
I'm building the entire labs repository in OSX too and I'm not able to reproduce that error. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/217#issuecomment-151792694
Re: [jclouds-labs] JCLOUDS-1009 JCLOUDS-1029: Fix tests and try to avoid deadlocks (#217)
Is this the problem we were having before? Is there a way I can see the log? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/217#issuecomment-151789344
Re: [jclouds-labs] JCLOUDS-1009 JCLOUDS-1029: Fix tests and try to avoid deadlocks (#217)
rebuild please --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/217#issuecomment-151787278
[jclouds-labs] JCLOUDS-1009 JCLOUDS-1029: Fix tests and try to avoid deadlocks (#217)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/217 -- Commit Summary -- * Fix testDirectory test * Ignore testBlobNameEscaping test * Try to avoid deadlocks on hibernate -- File Changes -- M jdbc/src/main/java/org/jclouds/jdbc/strategy/JdbcStorageStrategy.java (2) M jdbc/src/test/java/org/jclouds/jdbc/integration/HibernateHsqldbContainerIntegrationTest.java (7) M jdbc/src/test/resources/META-INF/persistence.xml (1) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/217.patch https://github.com/jclouds/jclouds-labs/pull/217.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/217
[jira] [Commented] (JCLOUDS-1029) JDBC tests hang the build
[
https://issues.apache.org/jira/browse/JCLOUDS-1029?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14977408#comment-14977408
]
Roman Coedo commented on JCLOUDS-1029:
--
I'll take a look at both ASAP.
> JDBC tests hang the build
> -
>
> Key: JCLOUDS-1029
> URL: https://issues.apache.org/jira/browse/JCLOUDS-1029
> Project: jclouds
> Issue Type: Bug
> Components: jclouds-labs
>Affects Versions: 1.9.1
>Reporter: Ignasi Barrera
> Labels: jdbc
>
> This is becoming a real issue, as more than 50% of the recent jclouds-labs
> and jclouds-labs-pull-requests Jenkins builds hang when running the JDBC
> tests.
> [~rcoedo] could you have a look at this and at JCLOUDS-1009?
> Here is an example stacktrace. Once it appears, the build hangs.
> {code}
> Test suite progress: tests succeeded: 59, failed: 0, skipped: 2.
> 0[pool-8-thread-2] ERROR org.hibernate.engine.jdbc.spi.SqlExceptionHelper
> - transaction rollback: serialization failure
> 0[pool-8-thread-1] ERROR org.hibernate.engine.jdbc.spi.SqlExceptionHelper
> - transaction rollback: serialization failure
> unable to delete container jenkins-blobstore-5256680054079839105, ignoring...
> unable to delete container jenkins-blobstore7, ignoring...
> javax.persistence.PersistenceException:
> org.hibernate.exception.LockAcquisitionException: could not extract ResultSet
> at
> org.hibernate.jpa.spi.AbstractEntityManagerImpl.convert(AbstractEntityManagerImpl.java:1763)
> at
> org.hibernate.jpa.spi.AbstractEntityManagerImpl.convert(AbstractEntityManagerImpl.java:1677)
> at
> org.hibernate.jpa.internal.QueryImpl.getSingleResult(QueryImpl.java:524)
> at
> org.jclouds.jdbc.repository.ContainerRepository.findContainerByName(ContainerRepository.java:40)
> at
> org.jclouds.jdbc.repository.ContainerRepository.deleteContainerByName(ContainerRepository.java:52)
> at
> org.jclouds.jdbc.service.JdbcService.deleteContainerByName(JdbcService.java:90)
> at
> com.google.inject.persist.jpa.JpaLocalTxnInterceptor.invoke(JpaLocalTxnInterceptor.java:70)
> at
> org.jclouds.jdbc.strategy.JdbcStorageStrategy.deleteContainer(JdbcStorageStrategy.java:167)
> at
> org.jclouds.blobstore.config.LocalBlobStore.deleteAndVerifyContainerGone(LocalBlobStore.java:740)
> at
> org.jclouds.blobstore.config.LocalBlobStore.deleteContainer(LocalBlobStore.java:208)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:606)
> at
> com.google.inject.internal.DelegatingInvocationHandler.invoke(DelegatingInvocationHandler.java:37)
> at com.sun.proxy.$Proxy48.deleteContainer(Unknown Source)
> at
> org.jclouds.blobstore.integration.internal.BaseBlobStoreIntegrationTest.deleteContainerOrWarnIfUnable(BaseBlobStoreIntegrationTest.java:188)
> at
> org.jclouds.blobstore.integration.internal.BaseBlobStoreIntegrationTest.access$1100(BaseBlobStoreIntegrationTest.java:69)
> at
> org.jclouds.blobstore.integration.internal.BaseBlobStoreIntegrationTest$11.run(BaseBlobStoreIntegrationTest.java:514)
> at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
> at java.util.concurrent.FutureTask.run(FutureTask.java:262)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at java.lang.Thread.run(Thread.java:745)
> Caused by: org.hibernate.exception.LockAcquisitionException: could not
> extract ResultSet
> at
> org.hibernate.exception.internal.SQLExceptionTypeDelegate.convert(SQLExceptionTypeDelegate.java:92)
> at
> org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:49)
> at
> org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:126)
> at
> org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:112)
> at
> org.hibernate.engine.jdbc.internal.ResultSetReturnImpl.extract(ResultSetReturnImpl.java:91)
> at org.hibernate.loader.Loader.getResultSet(Loader.java:2066)
> at org.hibernate.loader.Loader.executeQueryStatement(Loader.java:1863)
> at org.hibernate.loader.Loader.
Re: [jclouds] JCLOUDS-826: Remove filesystem (#846)
This PR was opened to discuss the removal of filesystem with the community, which was one of the points on my GSoC proposal. When I started the JDBC Blobstore one of the requisites was portability between different database technologies. Also, we decided to store the blobs in the database. Unfortunately, the blob storage apis offered by databases are not standard, so it's really hard to offer a generic jdbc blobstore without an object mapper. We ended up with something closer to a JPA Blobstore instead of a JDBC blobstore. IMHO jdbc blobstore and filesystem blobstore are different things and both should stay. Maybe in the future we can come up with an hybrid database which stores the blobs in the filesystem and the metadata in an embedded database. Storing only metadata in the database would make way easier to implement the jdbc calls in a generic way. It would be a better replacement. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/846#issuecomment-132174234
[jclouds] JCLOUDS-826: Remove filesystem (#846)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/846 -- Commit Summary -- * JCLOUDS-826: Remove filesystem -- File Changes -- M allblobstore/pom.xml (15) D apis/filesystem/README.txt (6) D apis/filesystem/pom.xml (120) D apis/filesystem/src/main/java/org/jclouds/filesystem/FilesystemApiMetadata.java (84) D apis/filesystem/src/main/java/org/jclouds/filesystem/config/FilesystemBlobStoreContextModule.java (50) D apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/FilesystemBlobKeyValidator.java (28) D apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/FilesystemContainerNameValidator.java (28) D apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorImpl.java (46) D apis/filesystem/src/main/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemContainerNameValidatorImpl.java (46) D apis/filesystem/src/main/java/org/jclouds/filesystem/reference/FilesystemConstants.java (33) D apis/filesystem/src/main/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImpl.java (865) D apis/filesystem/src/main/java/org/jclouds/filesystem/util/Utils.java (114) D apis/filesystem/src/main/java/org/jclouds/filesystem/util/internal/FileSystemBlobUtilsImpl.java (76) D apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemApiMetadataTest.java (28) D apis/filesystem/src/test/java/org/jclouds/filesystem/FilesystemBlobStoreTest.java (912) D apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemBlobIntegrationTest.java (88) D apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemContainerIntegrationTest.java (175) D apis/filesystem/src/test/java/org/jclouds/filesystem/integration/FilesystemServiceIntegrationTestDisabled.java (40) D apis/filesystem/src/test/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemBlobKeyValidatorTest.java (59) D apis/filesystem/src/test/java/org/jclouds/filesystem/predicates/validators/internal/FilesystemContainerNameValidatorTest.java (68) D apis/filesystem/src/test/java/org/jclouds/filesystem/strategy/internal/FilesystemStorageStrategyImplTest.java (669) D apis/filesystem/src/test/java/org/jclouds/filesystem/utils/TestUtils.java (227) D apis/filesystem/src/test/resources/logging.properties (27) M apis/pom.xml (1) M blobstore/src/main/clojure/org/jclouds/blobstore2.clj (2) -- Patch Links -- https://github.com/jclouds/jclouds/pull/846.patch https://github.com/jclouds/jclouds/pull/846.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/846
Re: [jclouds-labs] JCLOUDS-826: BaseServiceIntegrationTest for jdbc (#200)
Maybe it had something to do with a previous database file on your target folder. I'll check it just in case. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/200#issuecomment-131995257
[jclouds-site] Add JDBC user guide (#172)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-site/pull/172 -- Commit Summary -- * Add JDBC user guide -- File Changes -- M guides/index.md (1) A guides/jdbc.md (75) -- Patch Links -- https://github.com/jclouds/jclouds-site/pull/172.patch https://github.com/jclouds/jclouds-site/pull/172.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/172
[jclouds-labs] JCLOUDS-826: BaseServiceIntegrationTest for jdbc (#200)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/200 -- Commit Summary -- * JCLOUDS-826: BaseServiceIntegrationTest for jdbc -- File Changes -- M jdbc/src/main/java/org/jclouds/jdbc/strategy/JdbcStorageStrategy.java (31) A jdbc/src/test/java/org/jclouds/jdbc/integration/EclipselinkH2ServiceIntegrationTest.java (36) A jdbc/src/test/java/org/jclouds/jdbc/integration/HibernateHsqldbServiceIntegrationTest.java (36) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/200.patch https://github.com/jclouds/jclouds-labs/pull/200.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/200
Re: [jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
Seems like it's passing now! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/193#issuecomment-128504117
Re: [jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
Rebuild please --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/193#issuecomment-128503530
Re: [jclouds] Fix LocalBlobStore (#839)
This commit will fix the problems in the jdbc api and in the h2 provider. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/839#issuecomment-128334707
[jclouds] Fix LocalBlobStore (#839)
The ByteSource used to build the payload when using ranges was being recalculated for every range. This commit moves that logic outside the loop. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/839 -- Commit Summary -- * Fix LocalBlobStore -- File Changes -- M blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java (26) -- Patch Links -- https://github.com/jclouds/jclouds/pull/839.patch https://github.com/jclouds/jclouds/pull/839.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/839
Re: [jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
I may be using an old snapshot or maybe a new bug was introduced, but one test in the jdbc api is still failing (BaseBlobIntegrationTest.testGetTwoRanges). http://pastebin.com/JR8DcJZQ I'll take a look. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/193#issuecomment-127953800
Re: [jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
I knew there were some magic words but I couldn't remember them! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/193#issuecomment-127895576
Re: [jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
Can we rebuild? It should be good now. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/193#issuecomment-127885031
Re: [jclouds] JCLOUDS-979: Fix range get for LocalBlobStore (#837)
Tests and style pass on my machine, am I missing something? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/837#issuecomment-127567800
[jclouds] JCLOUDS-979: Fix range get for LocalBlobStore (#837)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds/pull/837 -- Commit Summary -- * JCLOUDS-979: Fix range get for LocalBlobStore -- File Changes -- M blobstore/src/main/java/org/jclouds/blobstore/config/LocalBlobStore.java (18) -- Patch Links -- https://github.com/jclouds/jclouds/pull/837.patch https://github.com/jclouds/jclouds/pull/837.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/837
[jira] [Commented] (JCLOUDS-979) BaseJdbcBlobStoreTest.testRanges failure
[
https://issues.apache.org/jira/browse/JCLOUDS-979?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14650379#comment-14650379
]
Roman Coedo commented on JCLOUDS-979:
-
This bug was introduced with this commit
https://github.com/jclouds/jclouds/commit/5ad245dea56b94735cc85c792a19c7121a0eb7dd
which assumes that every local blobstore returns blobs with a ByteArrayPayload
or a FilePayload. This is not the case in JdbcBlobStore.
Changing the way this blobstore handles the payloads is not trivial, since jpa
doesn't support mapping big blobs, we have to chunk every payload and have a
special stream which make database calls to ask for more chunks when needed.
> BaseJdbcBlobStoreTest.testRanges failure
>
>
> Key: JCLOUDS-979
> URL: https://issues.apache.org/jira/browse/JCLOUDS-979
> Project: jclouds
> Issue Type: Bug
> Components: jclouds-blobstore, jclouds-labs
>Affects Versions: 2.0.0
>Reporter: Andrew Gaul
>Assignee: Roman Coedo
>Priority: Minor
>
> {noformat}
> java.lang.ClassCastException: org.jclouds.jdbc.util.JdbcInputStream cannot be
> cast to com.google.common.io.ByteSource
> at
> org.jclouds.blobstore.config.LocalBlobStore.getBlob(LocalBlobStore.java:689)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:606)
> at
> com.google.inject.internal.DelegatingInvocationHandler.invoke(DelegatingInvocationHandler.java:37)
> at com.sun.proxy.$Proxy58.getBlob(Unknown Source)
> at
> org.jclouds.jdbc.BaseJdbcBlobStoreTest.testRanges(BaseJdbcBlobStoreTest.java:596)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:606)
> at
> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:84)
> at org.testng.internal.Invoker.invokeMethod(Invoker.java:714)
> at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:901)
> at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1231)
> at
> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:127)
> at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:111)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at java.lang.Thread.run(Thread.java:745)
> {noformat}
> From:
> https://jclouds.ci.cloudbees.com/job/jclouds-labs/1061/org.apache.jclouds.labs$jdbc/testReport/junit/org.jclouds.jdbc/EclipselinkH2BlobStoreTest/testRanges/
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
Re: [jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
About the failed tests, it's a problem with the jdbc api. Here is a link to the JIRA issue https://issues.apache.org/jira/browse/JCLOUDS-979 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/193#issuecomment-126926804
Re: [jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
> +
> +import java.util.Properties;
> +
> +import org.jclouds.jdbc.config.JdbcBlobStoreContextModule;
> +
> +import com.google.inject.persist.jpa.JpaPersistModule;
> +
> +public class H2JdbcBlobStoreContextModule extends JdbcBlobStoreContextModule
> {
> +
> + private static final String DEFAULT_FILE = "./jclouds-db";
> +
> + protected void configure() {
> + super.configure();
> +
> + Properties properties = new Properties();
> + properties.setProperty("hibernate.connection.url", "jdbc:h2:" +
> DEFAULT_FILE);
If I move it to the metadata, how can I inject it to the module? I tried so
many things to make this work but I just couldn't find a way to do it! :(
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/193/files#r35515364
[jclouds-labs] JCLOUDS-826: Add H2 provider (#193)
Simple jpa provider with a preconfigured embedded database. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/193 -- Commit Summary -- * JCLOUDS-826: Add H2 provider -- File Changes -- A h2-jdbc/pom.xml (158) A h2-jdbc/src/main/java/org/jclouds/h2/jdbc/H2JdbcApiMetadata.java (60) A h2-jdbc/src/main/java/org/jclouds/h2/jdbc/H2JdbcProviderMetadata.java (63) A h2-jdbc/src/main/java/org/jclouds/h2/jdbc/config/H2JdbcBlobStoreContextModule.java (38) A h2-jdbc/src/main/resources/META-INF/persistence.xml (43) A h2-jdbc/src/test/java/org/jclouds/h2/jdbc/H2JdbcProviderTest.java (29) A h2-jdbc/src/test/java/org/jclouds/h2/jdbc/blobstore/H2JdbcBlobIntegrationTest.java (34) A h2-jdbc/src/test/java/org/jclouds/h2/jdbc/blobstore/H2JdbcContainerIntegrationTest.java (34) A h2-jdbc/src/test/resources/log4j.properties (25) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/193.patch https://github.com/jclouds/jclouds-labs/pull/193.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/193
Re: [jclouds] JCLOUDS-930: Regression in a File System test. (#791)
I can take a look at this later today. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/791#issuecomment-117437852
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
Sorry, I rebased my own fork (doh!) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176#issuecomment-115854906
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
Thanks for the review @andrewgaul ! :) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176#issuecomment-112955508
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> @@ -0,0 +1,746 @@ > +/* This test is the equivalent of FilesystemBlobStoreTest. Some of the test methods are different from the ones included in the integration tests and since I wanted to be sure everything was working I took these too. Should I remove these? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176/files#r32677415
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + private List storeData(InputStream data) throws IOException {
> + ImmutableList.Builder chunks = ImmutableList.builder();
> + int bytes;
> + byte[] buffer = new byte[JdbcConstants.DEFAULT_CHUNK_SIZE];
> + while ((bytes = data.read(buffer, 0,
> JdbcConstants.DEFAULT_CHUNK_SIZE)) != -1) {
> + chunks.add(chunkRepository.create(new ChunkEntity(buffer,
> bytes)).getId());
> + buffer = new byte[JdbcConstants.DEFAULT_CHUNK_SIZE];
> + }
> + data.close();
> + return chunks.build();
> + }
> +
> + private void checkIntegrity(Blob blob, BlobEntity entity) throws
> IOException {
> + HashCode hash =
> blob.getMetadata().getContentMetadata().getContentMD5AsHashCode();
> + if (hash != null) {
> + HashingInputStream his = new HashingInputStream(Hashing.md5(), new
> JdbcInputStream(this, entity.getPayload().getChunks()));
I could make this explicit, but I think it's safe to leave it opened
(JdbcInputStream close method does nothing).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r32676697
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + @Transactional
> + public boolean blobExists(String containerName, String key) {
> + return findBlobById(containerName, key) != null;
> + }
> +
> + @Transactional(rollbackOn = IOException.class)
> + public BlobEntity createOrModifyBlob(String containerName, Blob blob,
> BlobAccess blobAccess) throws IOException {
> + String key = blob.getMetadata().getName();
> + Date creationDate = null;
> + BlobEntity oldBlobEntity = findBlobById(containerName, key);
> + if (oldBlobEntity != null) {
> + creationDate = oldBlobEntity.getCreationDate();
> + deleteBlob(containerName, key);
> + }
> + BlobEntity blobEntity = blobToBlobEntity.apply(blob);
> +
> blobEntity.getPayload().setChunks(storeData(blob.getPayload().openStream()));
The InputStream is closed in the storeData method :)
I think we need to check the integrity after the data is copied to the
database. I think we need to either set things up in the database to make it
return the hash of the copied data, or re-read the data from it and calculate
it. Any other ideas?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r32676447
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
I missed the headers on the new test class! It should be fixed now. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176#issuecomment-112213435
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + String key = blob.getMetadata().getName();
> + Date creationDate = null;
> + BlobEntity oldBlobEntity = findBlobById(containerName, key);
> + if (oldBlobEntity != null) {
> + creationDate = oldBlobEntity.getCreationDate();
> + deleteBlob(containerName, key);
> + }
> + BlobEntity blobEntity = blobToBlobEntity.apply(blob);
> +
> blobEntity.getPayload().setChunks(storeData(blob.getPayload().openStream()));
> +
> blobEntity.setContainerEntity(containerRepository.findContainerByName(containerName));
> + blobEntity.setKey(key);
> + blobEntity.setBlobAccess(blobAccess);
> + blobEntity.setCreationDate(creationDate);
> +
> +
> + HashCode hash =
> ByteStreams2.hashAndClose(blob.getPayload().openStream(), Hashing.md5());
A test fails if I don't do the integrity check. What's a good place to document
this behavior?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r32371719
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + public boolean equals(Object o) {
> + if (this == o)
> + return true;
> + if (o == null || getClass() != o.getClass())
> + return false;
> +
> + ContainerEntity containerEntity = (ContainerEntity) o;
> +
> + return containerAccess == containerEntity.containerAccess &&
> !(creationDate != null ?
> +!creationDate.equals(containerEntity.creationDate) :
> +containerEntity.creationDate != null) && !(id != null ?
> !id.equals(containerEntity.id) : containerEntity.id != null) && !(
> +name != null ? !name.equals(containerEntity.name) :
> containerEntity.name != null);
> + }
> +
> + @Override
> + public int hashCode() {
We didn't need these (I generated them by mistake) so I just deleted them.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r32371707
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + byte[] buffer = new byte[JdbcConstants.DEFAULT_CHUNK_SIZE];
> + while ((bytes = data.read(buffer, 0,
> JdbcConstants.DEFAULT_CHUNK_SIZE)) != -1) {
> + chunks.add(chunkRepository.create(new ChunkEntity(buffer,
> bytes)).getId());
> + buffer = new byte[JdbcConstants.DEFAULT_CHUNK_SIZE];
> + }
> + data.close();
> + return chunks.build();
> + }
> +
> + private void checkIntegrity(Blob blob, BlobEntity entity) throws
> IOException {
> + HashCode hash =
> blob.getMetadata().getContentMetadata().getContentMD5AsHashCode();
> + if (hash != null) {
> + HashingInputStream his = new HashingInputStream(Hashing.md5(), new
> JdbcInputStream(this, entity.getPayload().getChunks()));
> + HashCode actualHash = his.hash();
> + if (hash.equals(actualHash)) {
> +throw new IOException("MD5 hash code mismatch");
Some blob tests seem to expect IOException in this case
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r32371688
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
Thank you both for the review, I'll check these issues and fix them. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176#issuecomment-111043400
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + }
> + int b = currentChunk.getData()[bytesRead] & 0xff;
> + bytesRead = bytesRead + 1;
> + if (bytesRead >= currentChunk.getSize()) {
> + readNextChunk();
> + }
> + return b;
> + }
> +
> + private boolean hasFinished() {
> + return chunks.size() == 0 && bytesRead >= currentChunk.getSize();
> + }
> +
> + private void readNextChunk() {
> + if (chunks.size() > 0) {
> + this.currentChunk = jdbcService.findChunkById(chunks.get(0));
To properly implement a unit test for this class I'll probably need some
library to mock the jdbcService (e.g. mockito). Is this ok?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r32200142
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + @Test
> + public void testCreateContainerInLocation() {
> + assertThat(blobStore.createContainerInLocation(null,
> CONTAINER_NAME)).isTrue();
> + assertThat(blobStore.containerExists(CONTAINER_NAME)).isTrue();
> + }
> +
> + @Test
> + public void testDeleteContainer() {
> + assertThat(blobStore.createContainerInLocation(null,
> CONTAINER_NAME)).isTrue();
> + assertThat(blobStore.containerExists(CONTAINER_NAME)).isTrue();
> + blobStore.deleteContainer(CONTAINER_NAME);
> + assertThat(blobStore.containerExists(CONTAINER_NAME)).isFalse();
> + }
> +
> + @Test
> + public void testPutBlob() {
Yes, there are many putBlob/getBlob tests in the BaseJdbcStorageStrategyTest
and BaseJdbcBlobStoreTest classes.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r31865926
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
> + */
> +package org.jclouds.jdbc.entity;
> +
> +import com.google.common.collect.ImmutableList;
> +
> +import javax.persistence.ElementCollection;
> +import javax.persistence.Entity;
> +import javax.persistence.FetchType;
> +import javax.persistence.GeneratedValue;
> +import javax.persistence.GenerationType;
> +import javax.persistence.Id;
> +import java.util.Date;
> +import java.util.List;
> +
> +@Entity
> +public class PayloadEntity {
Is it possible? These are entities, not values. I think the ORM needs them to
be mutable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/176/files#r31476008
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
Rebuild please --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176#issuecomment-105985773
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
Now it's failing due to this `Found duplicate classes/resources in test classpath.`, but I need the duplicated classes for testing. Any idea on how could I solve it? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176#issuecomment-105769004
Re: [jclouds-labs] JCLOUDS-826: Add blob operations (#176)
Seems like I imported a java 8 package instead of the guava one. I fixed it and changed the IDE jdk now. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176#issuecomment-105766834
[jclouds-labs] JCLOUDS-826: Add blob operations (#176)
You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/176 -- Commit Summary -- * JCLOUDS-826: Add blob operations -- File Changes -- M jdbc/pom.xml (12) M jdbc/src/main/java/org/jclouds/jdbc/config/JPAInitializer.java (4) A jdbc/src/main/java/org/jclouds/jdbc/conversion/BlobEntityToBlob.java (82) A jdbc/src/main/java/org/jclouds/jdbc/conversion/BlobToBlobEntity.java (49) A jdbc/src/main/java/org/jclouds/jdbc/entity/BlobEntity.java (224) A jdbc/src/main/java/org/jclouds/jdbc/entity/BlobEntityPK.java (61) A jdbc/src/main/java/org/jclouds/jdbc/entity/ChunkEntity.java (74) R jdbc/src/main/java/org/jclouds/jdbc/entity/ContainerEntity.java (36) A jdbc/src/main/java/org/jclouds/jdbc/entity/PayloadEntity.java (198) M jdbc/src/main/java/org/jclouds/jdbc/predicates/validators/JdbcBlobKeyValidator.java (16) M jdbc/src/main/java/org/jclouds/jdbc/predicates/validators/JdbcContainerNameValidator.java (16) A jdbc/src/main/java/org/jclouds/jdbc/reference/JdbcConstants.java (29) A jdbc/src/main/java/org/jclouds/jdbc/repository/BlobRepository.java (54) A jdbc/src/main/java/org/jclouds/jdbc/repository/ChunkRepository.java (34) M jdbc/src/main/java/org/jclouds/jdbc/repository/ContainerRepository.java (24) M jdbc/src/main/java/org/jclouds/jdbc/service/JdbcService.java (208) M jdbc/src/main/java/org/jclouds/jdbc/strategy/JdbcStorageStrategy.java (127) M jdbc/src/main/java/org/jclouds/jdbc/util/JdbcBlobUtils.java (20) A jdbc/src/main/java/org/jclouds/jdbc/util/JdbcInputStream.java (67) A jdbc/src/test/java/org/jclouds/jdbc/BaseJdbcBlobStoreTest.java (746) A jdbc/src/test/java/org/jclouds/jdbc/EclipselinkH2BlobStoreTest.java (31) A jdbc/src/test/java/org/jclouds/jdbc/HibernateHsqldbBlobStoreTest.java (31) D jdbc/src/test/java/org/jclouds/jdbc/JdbcBlobStoreTest.java (72) A jdbc/src/test/java/org/jclouds/jdbc/integration/EclipselinkH2BlobIntegrationTest.java (35) A jdbc/src/test/java/org/jclouds/jdbc/integration/EclipselinkH2ContainerIntegrationTest.java (35) A jdbc/src/test/java/org/jclouds/jdbc/integration/HibernateHsqldbBlobIntegrationTest.java (35) A jdbc/src/test/java/org/jclouds/jdbc/integration/HibernateHsqldbContainerIntegrationTest.java (35) A jdbc/src/test/java/org/jclouds/jdbc/strategy/BaseJdbcStorageStrategyTest.java (212) A jdbc/src/test/java/org/jclouds/jdbc/strategy/EclipselinkH2StorageStrategyTest.java (31) A jdbc/src/test/java/org/jclouds/jdbc/strategy/HibernateHsqldbStorageStrategyTest.java (31) D jdbc/src/test/java/org/jclouds/jdbc/strategy/JdbcStorageStrategyTest.java (101) A jdbc/src/test/java/org/jclouds/jdbc/validators/JdbcBlobKeyValidatorTest.java (47) A jdbc/src/test/java/org/jclouds/jdbc/validators/JdbcContainerNameValidatorTest.java (56) M jdbc/src/test/resources/META-INF/persistence.xml (34) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/176.patch https://github.com/jclouds/jclouds-labs/pull/176.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/176
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + public boolean createContainerInLocation(String name, Location location,
> + CreateContainerOptions createContainerOptions) {
> + logger.debug("Creating container %s", name);
> + jdbcContainerNameValidator.validate(name);
> + containerRepository.create(Container.builder().name(name).build());
> + return true;
> + }
> +
> + @Override
> + public ContainerAccess getContainerAccess(String name) {
> + return ContainerAccess.PRIVATE;
> + }
> +
> + @Override
> + public void setContainerAccess(String s, ContainerAccess containerAccess)
> {
> + // Unsupported
:+1: I'm on it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/173/files#r29955010
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + true > + > + > + com.google.auto.value > + auto-value > + 1.0 > + > + > + com.google.inject.extensions > + guice-persist > + 4.0 > + > + > + org.hibernate.javax.persistence > + hibernate-jpa-2.1-api > + 1.0.0.Final The 2.1 javax.persistence api is part of Java EE. It is available as a standalone package but the last version is 1.0.2. Hibernate and other providers offer a 2.1 api and the implementation as two different packages. I added the hibernate jpa api as a dependency, The entity manager (the implementation) is test scoped. Thoughts? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/173/files#r29954669
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + List containers = containerRepository.findAllContainers();
> + ImmutableList.Builder result = ImmutableList.builder();
> + for (Container c : containers) {
> + result.add(c.getName());
> + }
> + return result.build();
> + }
> +
> + @Override
> + @Transactional
> + public boolean createContainerInLocation(String name, Location location,
> + CreateContainerOptions createContainerOptions) {
> + logger.debug("Creating container %s", name);
> + jdbcContainerNameValidator.validate(name);
> + containerRepository.create(Container.builder().name(name).build());
> + return true;
I can return false when a persistence exception raises, but I will have to make
a service layer to handle the transactions then. I'll fix this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/173/files#r29954036
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + > +import com.google.common.collect.ImmutableSet; > +import com.google.inject.Guice; > +import com.google.inject.Module; > +import com.google.inject.persist.jpa.JpaPersistModule; > +import org.jclouds.blobstore.domain.StorageMetadata; > +import org.jclouds.blobstore.domain.StorageType; > +import org.jclouds.jdbc.module.TestContextModule; > +import org.testng.annotations.BeforeMethod; > +import org.testng.annotations.Test; > + > +import java.util.Date; > + > +import static org.assertj.core.api.Assertions.assertThat; > + > +@Test(groups = "unit", testName = "JdbcStorageStrategyTest", singleThreaded > = true) Same as above. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/173/files#r29954186
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> +import com.google.common.collect.ImmutableSet; > +import com.google.inject.Module; > +import com.google.inject.persist.jpa.JpaPersistModule; > +import org.jclouds.ContextBuilder; > +import org.jclouds.blobstore.BlobStore; > +import org.jclouds.blobstore.BlobStoreContext; > +import org.testng.annotations.AfterMethod; > +import org.testng.annotations.BeforeMethod; > +import org.testng.annotations.Test; > + > +import java.io.IOException; > +import java.util.Set; > + > +import static org.assertj.core.api.Assertions.assertThat; > + > +@Test(groups = "unit", testName = "JdbcBlobStoreTest", singleThreaded = true) It does, the methods are running against the same database. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/173/files#r29954166
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + super(new Builder());
> + }
> +
> + protected JdbcApiMetadata(Builder builder) {
> + super(builder);
> + }
> +
> + public static class Builder extends BaseApiMetadata.Builder {
> +
> + protected Builder() {
> + id("jdbc")
> + .name("Jdbc BlobStore")
> + .identityName("Unused")
> + .defaultEndpoint("http://localhost/transient";)
> + .defaultIdentity("unused")
> + .defaultCredential("unused")
I get an error if the identity is not present in properties.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/173/files#r29952536
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > +package org.jclouds.jdbc; > + > +import com.google.auto.service.AutoService; > +import com.google.common.collect.ImmutableSet; > +import com.google.inject.Module; > +import org.jclouds.apis.ApiMetadata; This one is used in AutoService(ApiMetadata.class) too, not just for the javadoc. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/173/files#r29951495
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + }
> +
> + public JdbcApiMetadata() {
> + super(new Builder());
> + }
> +
> + protected JdbcApiMetadata(Builder builder) {
> + super(builder);
> + }
> +
> + public static class Builder extends BaseApiMetadata.Builder {
> +
> + protected Builder() {
> + id("jdbc")
> + .name("Jdbc BlobStore")
> + .identityName("Unused")
The datasource config belongs in a persistence.xml file. The persistence unit
must be passed as a JpaPersistModule to the blobstore.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/173/files#r29951753
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + public boolean createContainerInLocation(String name, Location location,
> + CreateContainerOptions createContainerOptions) {
> + logger.debug("Creating container %s", name);
> + jdbcContainerNameValidator.validate(name);
> + containerRepository.create(Container.builder().name(name).build());
> + return true;
> + }
> +
> + @Override
> + public ContainerAccess getContainerAccess(String name) {
> + return ContainerAccess.PRIVATE;
> + }
> +
> + @Override
> + public void setContainerAccess(String s, ContainerAccess containerAccess)
> {
> + // Unsupported
What difference does it make if a container is marked on the database as
private/public? I can store it in the database, but it seems pretty useless to
me since the container can't be shared to other database users this way like,
let's say, a S3 bucket. Again, probably I am missing something :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/173/files#r29950838
Re: [jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
> + public boolean createContainerInLocation(String name, Location location,
> + CreateContainerOptions createContainerOptions) {
> + logger.debug("Creating container %s", name);
> + jdbcContainerNameValidator.validate(name);
> + containerRepository.create(Container.builder().name(name).build());
> + return true;
> + }
> +
> + @Override
> + public ContainerAccess getContainerAccess(String name) {
> + return ContainerAccess.PRIVATE;
> + }
> +
> + @Override
> + public void setContainerAccess(String s, ContainerAccess containerAccess)
> {
> + // Unsupported
Since the database can't make public rows, I'm returning
ContainerAccess.PRIVATE for all the containers and never changing it. Maybe I'm
missing something here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/173/files#r29918208
[jclouds-labs] JCLOUDS-826: Add jdbc api skeleton (#173)
First commit for the new jdbc api. Contains the skeleton and the container operations. You can view, comment on, or merge this pull request online at: https://github.com/jclouds/jclouds-labs/pull/173 -- Commit Summary -- * JCLOUDS-826: Add skeleton and container operations -- File Changes -- A jdbc/.gitignore (1) A jdbc/pom.xml (155) A jdbc/src/main/java/org/jclouds/jdbc/JdbcApiMetadata.java (73) A jdbc/src/main/java/org/jclouds/jdbc/config/JPAInitializer.java (29) A jdbc/src/main/java/org/jclouds/jdbc/config/JdbcBlobStoreContextModule.java (44) A jdbc/src/main/java/org/jclouds/jdbc/entity/Container.java (94) A jdbc/src/main/java/org/jclouds/jdbc/predicates/validators/JdbcBlobKeyValidator.java (38) A jdbc/src/main/java/org/jclouds/jdbc/predicates/validators/JdbcContainerNameValidator.java (38) A jdbc/src/main/java/org/jclouds/jdbc/repository/ContainerRepository.java (42) A jdbc/src/main/java/org/jclouds/jdbc/repository/GenericRepository.java (55) A jdbc/src/main/java/org/jclouds/jdbc/strategy/JdbcStorageStrategy.java (191) A jdbc/src/main/java/org/jclouds/jdbc/util/JdbcBlobUtils.java (72) A jdbc/src/test/java/org/jclouds/jdbc/JdbcApiMetadataTest.java (28) A jdbc/src/test/java/org/jclouds/jdbc/JdbcBlobStoreTest.java (72) A jdbc/src/test/java/org/jclouds/jdbc/module/TestContextModule.java (29) A jdbc/src/test/java/org/jclouds/jdbc/strategy/JdbcStorageStrategyTest.java (75) A jdbc/src/test/resources/META-INF/persistence.xml (23) A jdbc/src/test/resources/log4j.properties (25) M pom.xml (1) -- Patch Links -- https://github.com/jclouds/jclouds-labs/pull/173.patch https://github.com/jclouds/jclouds-labs/pull/173.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/173
Re: [jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes using @AutoValue (#64)
Thanks for the review @adriancole ! I'll address these issues as soon as possible, glad to help. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/64#issuecomment-61139102
[jclouds-labs-aws] JCLOUDS-750: Refactor glacier domain classes using @AutoValue (#64)
You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-labs-aws autovalue Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-labs-aws/pull/64 -- Commit Summary -- * JCLOUDS-750: Add @AutoValue to pom.xml and refactor VaultMetadata * JCLOUDS-750: Refactor PartMetadata using @AutoValue * JCLOUDS-750: Refactor some collections using @AutoValue * JCLOUDS-750: Refactor JobMetadata using @AutoValue * JCLOUDS-750: Refactor GlacierError using @AutoValue * JCLOUDS-750: Refactor ArchiveMetadata using @AutoValue -- File Changes -- M glacier/pom.xml (6) M glacier/src/main/java/org/jclouds/glacier/domain/ArchiveMetadata.java (82) M glacier/src/main/java/org/jclouds/glacier/domain/ArchiveMetadataCollection.java (35) M glacier/src/main/java/org/jclouds/glacier/domain/GlacierError.java (56) M glacier/src/main/java/org/jclouds/glacier/domain/JobMetadata.java (176) M glacier/src/main/java/org/jclouds/glacier/domain/MultipartUploadMetadata.java (111) M glacier/src/main/java/org/jclouds/glacier/domain/PaginatedJobCollection.java (34) M glacier/src/main/java/org/jclouds/glacier/domain/PaginatedMultipartUploadCollection.java (30) M glacier/src/main/java/org/jclouds/glacier/domain/PaginatedVaultCollection.java (32) M glacier/src/main/java/org/jclouds/glacier/domain/PartMetadata.java (28) M glacier/src/main/java/org/jclouds/glacier/domain/VaultMetadata.java (96) -- Patch Links -- https://github.com/jclouds/jclouds-labs-aws/pull/64.patch https://github.com/jclouds/jclouds-labs-aws/pull/64.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/64
Re: [jclouds-labs-aws] Use jclouds closeQuietly instad of guava's (#63)
oops, sorry about that! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/63#issuecomment-61130358
[jclouds-labs-aws] Use jclouds closeQuietly instad of guava's (#63)
You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-labs-aws closequietly Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-labs-aws/pull/63 -- Commit Summary -- * Use jclouds closeQuietly instad of guava's -- File Changes -- M glacier/src/main/java/org/jclouds/glacier/util/AWSRequestSignerV4.java (3) M glacier/src/main/java/org/jclouds/glacier/util/TreeHash.java (3) -- Patch Links -- https://github.com/jclouds/jclouds-labs-aws/pull/63.patch https://github.com/jclouds/jclouds-labs-aws/pull/63.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/63
Re: [jclouds-site] Announce Google Summer of Code 2014 results (#125)
+1 from me too. What do you think @hsbhathiya, should we add a photo? :) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/125#issuecomment-56683195
Re: [jclouds] Add copyBlob to portable abstraction and add S3-optimized variant (#511)
What kind of options do you have in mind @andrewgaul? I can make this work with glacier. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/511#issuecomment-54723119
[jira] [Resolved] (JCLOUDS-457) Amazon Glacier support
[ https://issues.apache.org/jira/browse/JCLOUDS-457?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Roman Coedo resolved JCLOUDS-457. - Resolution: Fixed Fix Version/s: 1.8.0 > Amazon Glacier support > -- > > Key: JCLOUDS-457 > URL: https://issues.apache.org/jira/browse/JCLOUDS-457 > Project: jclouds > Issue Type: New Feature > Components: jclouds-blobstore >Reporter: Andrew Gaul > Assignee: Roman Coedo > Labels: gsoc2014 > Fix For: 1.8.0 > > > jclouds should support Amazon Glacier which provides a different > price/availability trade off than traditional blobstores. Note that this > will require some thought for how to handle blob reads. -- This message was sent by Atlassian JIRA (v6.2#6252)
[jira] [Created] (JCLOUDS-659) Add support for multiple regions to Amazon Glacier AWSRequestSignerV4
Roman Coedo created JCLOUDS-659: --- Summary: Add support for multiple regions to Amazon Glacier AWSRequestSignerV4 Key: JCLOUDS-659 URL: https://issues.apache.org/jira/browse/JCLOUDS-659 Project: jclouds Issue Type: New Feature Components: jclouds-labs-aws Affects Versions: 1.8.0 Reporter: Roman Coedo Amazon Glacier introduced a signer for AWS Signature Version 4. Right now, the region string used for signing the requests is defined statically, making imposible to sign requests to other regions than us-east-1. -- This message was sent by Atlassian JIRA (v6.2#6252)
Re: [jclouds-examples] Add glacier example (#58)
> +
> + // Create a container
> + final String containerName = UUID. randomUUID().toString() +
> "_interruptionExample";
> + blobstore.createContainerInLocation(null, containerName); // Create a
> vault
> +
> + // Create a blob
> + ByteSource payload = ByteSource.wrap("data".getBytes(Charsets.UTF_8));
> + Blob blob = blobstore.blobBuilder("ignored") // The blob name is
> ignored in Glacier
> +.payload(payload)
> +.contentLength(payload.size())
> +.build();
> +
> + // Put the blob in the container
> + final String blobId = blobstore.putBlob(containerName, blob);
> +
> + // New thread
Wouldn't be executors useless here? In this case I'm not going to reuse the
thread.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/58/files#r16032757
Re: [jclouds-examples] Add glacier example (#58)
> +return false;
> + default:
> +System.out.println("Not a valid option");
> +break;
> + }
> + }
> + catch(InputMismatchException e){
> + System.out.println("Not a valid option");
> + }
> + return true;
> + }
> +
> + private static ByteSource buildData(long size) {
> + byte[] array = new byte[1024];
> + Arrays.fill(array, (byte) 'a');
> + return ByteSources.repeatingArrayByteSource(array).slice(0, size);
Is this part of the new SNAPSHOT version? I can't import the TestUtils class
while using 1.8.0 version.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-examples/pull/58/files#r16032611
[jclouds-labs-aws] JCLOUDS-457: Extend javadoc and add README.md (#56)
Added more javadoc documentation. This covers the basics of the provider. You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-labs-aws readme Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-labs-aws/pull/56 -- Commit Summary -- * JCLOUDS-457: Extend javadoc and add README.md -- File Changes -- A glacier/README.md (7) M glacier/src/main/java/org/jclouds/glacier/GlacierClient.java (68) -- Patch Links -- https://github.com/jclouds/jclouds-labs-aws/pull/56.patch https://github.com/jclouds/jclouds-labs-aws/pull/56.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/56
Re: [jclouds-site] Add Glacier guide (#121)
Thanks for the review @demobox, I'll fix these as soon as possible. > Do you want to include the Known Issues and Resources you had included in the > README PR? I was planning to leave these two in the README.md to have a similar style as the other providers, what do you think about this? If it fits the guide better, I can do it. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/121#issuecomment-51341206
Re: [jclouds-site] Add Glacier guide (#121)
> +.build();
> +String key = blobstore.putBlob(containerName, blob);
> +
> +// when you need access to glacier specific features,
> +// use the provider context
> +RestContext restContext =
> context.unwrap();
> +GlacierClient client = GlacierClient.class.cast(context.getProviderSpecifi)
> +
> +JobRequest archiveRetrievalJobRequest = ArchiveRetrievalJobRequest.builder()
> +.archiveId(archiveId)
> +.description("retrieval job")
> +.build();
> +String jobId = client.initiateJob(vaultName, archiveRetrievalJobRequest);
> +
> +// Retrieve output when the job is done
> +Paylaoad result = client.getJobOutput(vaultName, jobId);
It should, it's taken from the jclouds-examples and those were working. I
probably introduced the typo when writing the guide.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-site/pull/121/files#r15876212
Re: [jclouds-site] Add Glacier guide (#121)
> +* `containerExists` This is a regular request. > + > +* `createContainerInLocation` This is a regular request. Note that the > location is currently not supported. > + > +# Blob operations > +* `list` An inventory will be retrieved to obtain the list. Note that this > will take several hours and the result may be inaccurate (Inventories are > updated every 24 hours). > + > +* `blobMetadata` An inventory will be retrieved to obtain the blob list and > the method will iterate through it. This operation will take several hours > and the result may be inaccurate (Inventories are updated every 24 hours). > + > +* `blobExists` This operation calls blobMetadata, check blobMetadata > operation for more information. > + > +* `getBlob` This operation will take more than 4 hours. > + > +* `removeBlob` This is a regular request. > + > +* `putBlob` This is a regular request. See the answer above, I think I can remove this and extend the javadoc instead. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/121/files#r15876044
Re: [jclouds-site] Add Glacier guide (#121)
> +* `listParts` You can list the parts of an ongoing multipart upload at any > time. By default it returns up to 1,000 uploaded parts, but you can control > this using the request options. > + > +* `listMultipartUploads` You can list the ongoing multipart uploads for a > specific vault. By default, this operation returns up to 1,000 multipart > uploads. You can control this using the request options. > + > +**Note**: Due to an issue with java 6, only parts up to 1GB are supported > using uploadPart operation. This will be fixed once jclouds drops support for > java 6. > + > +# Job operations > +* `initiateJob` Initiates a job. The job can be an inventory retrieval or an > archive retrieval. Once the job is started the estimated time to complete it > is ~4-5 hours. See > [this](http://docs.aws.amazon.com/amazonglacier/latest/dev/api-initiate-job-post.html) > for more detailed information about the available options for both jobs. > + > +* `describeJob` Retrieves information about an ongoing job. Among the > information you will find the initiatino date, the user who initiated the job > or the status message. > + > +* `listJobs` Lists the ongoing jobs and the recently finished jobs for a > vault. By default this operation returns up to 1,000 jobs in the response, > but you can control this using the request options. Note that this operation > also returns the recently finished jobs and you can still download their > output. > + > +* `getJobOutput` Gets the raw job output of any kind of job. You can > download the job output within the 24 hour period after Glacier comlpetes a > job. > + > +* `getInventoryRetrievalOutput` Gets the job output for the given job ID. > The job must be an inventory retrieval, otherwise this operation will fail. > This operation will parse the job output and build a collection of objects > for you. > Do we need all these above? They look very similar to what you'd expect in > the Javadoc. You are right about this, I think I should remove this and extend the javadoc instead. > Would it make more sense to have more examples here? > I'd wait for now to see if somebody asks for something more specific in the > mail list. I'd like to have examples covering the most common questions, but > for now I think this basic example and the examples in jclouds-examples are > enough. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/121/files#r15875815
Re: [jclouds-site] Add Glacier guide (#121)
> @@ -0,0 +1,146 @@ > +--- > +layout: page > +title: "Glacier: In Depth" Since I added it to the In depth section I used the according title, e.g. http://jclouds.apache.org/guides/aws-ec2/ or http://jclouds.apache.org/guides/aws-s3/ --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/121/files#r15874742
Re: [jclouds-labs-aws] JCLOUDS-457: Add README.md (#54)
I've extended this documentation with a small example snippet and I added a guide in the In-depth section [here](http://jclouds.apache.org/guides/) This is the PR: https://github.com/jclouds/jclouds-site/pull/121 --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/54#issuecomment-51174192
[jclouds-site] Add Glacier guide (#121)
You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-site glacierguide Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-site/pull/121 -- Commit Summary -- * Add Glacier guide -- File Changes -- A guides/glacier.md (143) M guides/index.md (3) -- Patch Links -- https://github.com/jclouds/jclouds-site/pull/121.patch https://github.com/jclouds/jclouds-site/pull/121.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/121
[jclouds-site] Fix links in /start/compute (#120)
Fixed the broken "Compute supported providers" link. You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-site patch-2 Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-site/pull/120 -- Commit Summary -- * Fix links in /start/compute -- File Changes -- M start/compute.md (2) -- Patch Links -- https://github.com/jclouds/jclouds-site/pull/120.patch https://github.com/jclouds/jclouds-site/pull/120.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/120
[jclouds-site] Fix links in /start/blobstore (#119)
Fixed the broken "ComputeGuide" and "Blobstore supported providers" links. You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-site patch-1 Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-site/pull/119 -- Commit Summary -- * Fix links in /start/blobstore -- File Changes -- M start/blobstore.md (4) -- Patch Links -- https://github.com/jclouds/jclouds-site/pull/119.patch https://github.com/jclouds/jclouds-site/pull/119.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/119
Re: [jclouds-labs-aws] JCLOUDS-457: Overloading getBlob (#55)
There we go, thanks @demobox! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/55#issuecomment-51128923
[jclouds-examples] Add glacier example (#58)
You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-examples master Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-examples/pull/58 -- Commit Summary -- * Add glacier example -- File Changes -- A glacier/README.md (17) A glacier/pom.xml (83) A glacier/src/main/assembly/jar-with-dependencies.xml (42) A glacier/src/main/java/org/jclouds/examples/glacier/MainApp.java (255) -- Patch Links -- https://github.com/jclouds/jclouds-examples/pull/58.patch https://github.com/jclouds/jclouds-examples/pull/58.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-examples/pull/58
Re: [jclouds-labs-aws] JCLOUDS-457: Overloading getBlob (#55)
Unfortunately we don't have any tests for the glacier blobstore yet. I tried to adapt the base blobstore tests to the glacier implementation but it just doesn't fit, it would take days to run those. Obviously this is something that needs to be worked out, for now, I've tested the class manually with the blobstore CLI. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/55#issuecomment-51040169
Re: [jclouds-labs-aws] JCLOUDS-457: Add README.md (#54)
> The main reason I was suggesting a Getting Started Guide rather than a README > is that most of the READMEs for APIs and providers are very stubby and/or not > very useful, and the result is that users are simply unlikely to look there. I agree with this, maybe this is something we need to improve? > Once I've had a chance to review the content, I'll also perhaps understand > better why you feel it wouldn't fit there ;-) In that case, perhaps a longer > Javadoc for GlacierClient and/or GlacierBlobstore might be a good option? Some of the documentation is already in the javadoc and other parts are covered by amazon's documentation. In any case I think this is a nice recap for someone who wants to find more about this provider and its implementation. Back when I started with Glacier, something like this for S3, Atmos or Azureblob would have been really helpful. In any case, I think we should discuss this once you review the content. I'd like to know your opinion. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/54#issuecomment-51036370
Re: [jclouds-site] Release Notes for jclouds 1.8.0 (#114)
> + > +## Introduction > + > +You can read the official announcement at [Apache jclouds 1.8.0 > released](TODO). You can read the details of the specific JIRA issues > addressed in this release at the [JIRA Release > Notes](https://issues.apache.org/jira/secure/ReleaseNote.jspa?version=12325587&styleName=Html&projectId=12314430). > + > +To get jclouds, please see the [jclouds installation guide](/start/install/). > + > +**Note: 1.8.0 will be the last major version of jclouds to support Java 6** > + > +## Highlights > + > +* TODO: something about SoftLayer? > +* TODO: what's new in OpenStack? > +* TODO: what's new in Rackspace? > +* TODO: announce the version of Guava (16, right)? > +* TODO: other providers > "jclouds now supports the Amazon Glacier API, directly through the > GlacierClient and through the Blobstore view. See the Getting Started Guide > for Glacier for more information" Seems right to me. > Do we regard the implementation as complete or still a work-in-progress, by > the way? The implementation is complete but there is a wide margin for improvement (like using simple notification service instead of polling to check the job status), so it depends on what work in progress means. I would be fine with both. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-site/pull/114/files#r15746065
Re: [jclouds-labs-aws] JCLOUDS-457: Add README.md (#54)
> Do you think it would make more sense to reformat this as a Getting Started > Guide on the site? Sorry to change my mind on this, but I checked the other guides on the site and I think this wouldn't fit there. In my humble opinion, the Glacier getting started belongs [here](http://jclouds.apache.org/guides/aws/), since the procedure to start coding is identical to s3 and ec2. I can extend that page with the glacier information. This is a little more detailed guide on how things work, explaining some tricky details about glacier that are not very well documented in the Amazon documentation and some specific problems with our implementation. @demobox @andrewgaul any thoughts? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/54#issuecomment-51006515
[jclouds-labs-aws] JCLOUDS-457: Overloading getBlob (#55)
The missing getBlob implementation has been added. You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-labs-aws fix Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-labs-aws/pull/55 -- Commit Summary -- * JCLOUDS-457: Overloading getBlob -- File Changes -- M glacier/src/main/java/org/jclouds/glacier/blobstore/GlacierBlobStore.java (15) -- Patch Links -- https://github.com/jclouds/jclouds-labs-aws/pull/55.patch https://github.com/jclouds/jclouds-labs-aws/pull/55.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/55
Re: [jclouds-labs-aws] JCLOUDS-457: Add README.md (#54)
I agree with you, one place will be easier to maintain. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/54#issuecomment-50985265
Re: [jclouds-labs-aws] JCLOUDS-457: Add polling strategy (#44)
> + }
> +
> + @Inject
> + public BasePollingStrategy(GlacierClient client) {
> + this(DEFAULT_INITIAL_WAIT, DEFAULT_TIME_BETWEEN_POLLS, client);
> + }
> +
> + private boolean inProgress(String job, String vault) {
> + JobMetadata jobMetadata = client.describeJob(vault, job);
> + return (jobMetadata != null) && (jobMetadata.getStatusCode() ==
> JobStatus.IN_PROGRESS);
> + }
> +
> + private void waitForJob(String job, String vault) throws
> InterruptedException {
> + Thread.sleep(initialWait);
> + while (inProgress(job, vault)) {
> + Thread.sleep(timeBetweenPolls);
Seems right! thanks @demobox
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/44/files#r15733544
Re: [jclouds-labs-aws] JCLOUDS-457: Add polling strategy (#44)
> + }
> +
> + @Inject
> + public BasePollingStrategy(GlacierClient client) {
> + this(DEFAULT_INITIAL_WAIT, DEFAULT_TIME_BETWEEN_POLLS, client);
> + }
> +
> + private boolean inProgress(String job, String vault) {
> + JobMetadata jobMetadata = client.describeJob(vault, job);
> + return (jobMetadata != null) && (jobMetadata.getStatusCode() ==
> JobStatus.IN_PROGRESS);
> + }
> +
> + private void waitForJob(String job, String vault) throws
> InterruptedException {
> + Thread.sleep(initialWait);
> + while (inProgress(job, vault)) {
> + Thread.sleep(timeBetweenPolls);
Do you think I should do this in the javadoc, or maybe in the readme/getting
started guide?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/44/files#r15729458
Re: [jclouds-labs-aws] JCLOUDS-457: Add README.md (#54)
I want to make an example in jclouds-examples and I think I can make a better Getting Started Guide using some of its code. I can reuse part of this README to make the Getting Started guide though. We will have both : ) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/54#issuecomment-50964987
[jclouds-labs-aws] JCLOUDS-457: Add README.md (#54)
Added a small documentation. This covers the basics of the provider and the blobstore. You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-labs-aws documentation Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-labs-aws/pull/54 -- Commit Summary -- * JCLOUDS-457: Add README.md -- File Changes -- A glacier/README.md (122) -- Patch Links -- https://github.com/jclouds/jclouds-labs-aws/pull/54.patch https://github.com/jclouds/jclouds-labs-aws/pull/54.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/54
[jclouds-labs-aws] JCLOUDS-457: Fix some nits (#53)
Small refactor fixing some style issues. You can merge this Pull Request by running: git pull https://github.com/rcoedo/jclouds-labs-aws fix Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds-labs-aws/pull/53 -- Commit Summary -- * JCLOUDS-457: Fix some nits -- File Changes -- M glacier/src/main/java/org/jclouds/glacier/GlacierClient.java (5) M glacier/src/main/java/org/jclouds/glacier/blobstore/GlacierBlobStore.java (22) M glacier/src/main/java/org/jclouds/glacier/blobstore/functions/ArchiveMetadataCollectionToStorageMetadata.java (19) D glacier/src/main/java/org/jclouds/glacier/blobstore/functions/ArchiveMetadataToBlobMetadata.java (41) M glacier/src/main/java/org/jclouds/glacier/blobstore/strategy/internal/BasePollingStrategy.java (14) -- Patch Links -- https://github.com/jclouds/jclouds-labs-aws/pull/53.patch https://github.com/jclouds/jclouds-labs-aws/pull/53.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/53
Re: [jclouds-labs-aws] JCLOUDS-457: Fix some nits (#53)
Thanks @demobox for the reviews! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/53#issuecomment-50958227
Re: [jclouds-labs-aws] JCLOUDS-457: Add polling strategy (#44)
> @@ -112,11 +111,8 @@ public void testListJobs() {
>
> @Test(groups = {"live", "livelong", "longtest"}, dependsOnMethods =
> {"testInitiateJob", "testDescribeJob", "testListJobs"})
> public void testWaitForSucceed() throws InterruptedException {
> - Thread.sleep(INITIAL_WAIT);
> - while (api.describeJob(VAULT_NAME,
> archiveRetrievalJob).getStatusCode() == JobStatus.IN_PROGRESS ||
> -api.describeJob(VAULT_NAME,
> inventoryRetrievalJob).getStatusCode() == JobStatus.IN_PROGRESS) {
> - Thread.sleep(TIME_BETWEEN_POLLS);
> - }
> + new BasePollingStrategy(api).waitForSuccess(VAULT_NAME,
> archiveRetrievalJob);
It does, that test is part of the long test group and has its own test profile.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/44/files#r15728170
Re: [jclouds-labs-aws] JCLOUDS-457: Add polling strategy (#44)
> + this.initialWait = initialWait;
> + this.timeBetweenPolls = timeBetweenPolls;
> + this.client = checkNotNull(client, "client");
> + }
> +
> + @Inject
> + public BasePollingStrategy(GlacierClient client) {
> + this(DEFAULT_INITIAL_WAIT, DEFAULT_TIME_BETWEEN_POLLS, client);
> + }
> +
> + private boolean inProgress(String job, String vault) {
> + JobMetadata jobMetadata = client.describeJob(vault, job);
> + return (jobMetadata != null) && (jobMetadata.getStatusCode() ==
> JobStatus.IN_PROGRESS);
> + }
> +
> + private void waitForJob(String job, String vault) throws
> InterruptedException {
Just clarity, but I can inline it too.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/44/files#r15728167
Re: [jclouds-labs-aws] JCLOUDS-457: Add polling strategy (#44)
> + }
> +
> + @Inject
> + public BasePollingStrategy(GlacierClient client) {
> + this(DEFAULT_INITIAL_WAIT, DEFAULT_TIME_BETWEEN_POLLS, client);
> + }
> +
> + private boolean inProgress(String job, String vault) {
> + JobMetadata jobMetadata = client.describeJob(vault, job);
> + return (jobMetadata != null) && (jobMetadata.getStatusCode() ==
> JobStatus.IN_PROGRESS);
> + }
> +
> + private void waitForJob(String job, String vault) throws
> InterruptedException {
> + Thread.sleep(initialWait);
> + while (inProgress(job, vault)) {
> + Thread.sleep(timeBetweenPolls);
It seemed correct to me allowing interruption. Also, if the user works with the
provider instead of the abstraction and uses the PollingStrategy, they can
handle the interruption as they please.
Any thoughts about this @andrewgaul ?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/44/files#r15728156
Re: [jclouds-labs-aws] JCLOUDS-457: Add list operation (#46)
> @@ -109,7 +125,7 @@ public boolean blobExists(String container, String key) {
>
> @Override
> public String putBlob(String container, Blob blob) {
> - return sync.uploadArchive(container, blob.getPayload(),
> blob.getMetadata().getName());
> + return sync.uploadArchive(container, blob.getPayload());
As far as I know the archive IDs are always the same length and are set by
Amazon. An example would be:
>TJgHcrOSfAkV6hdPqOATYfp_0ZaxL1pIBOc02iZ0gDPMr2ig-nhwd_PafstsdIf6HSrjHnP-3p6LCJClYytFT_CBhT9CwNxbRaM5MetS3I-GqwxI3Y8QtgbJbhEQPs0mJ3KExample
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/46/files#r15728063
Re: [jclouds-labs-aws] JCLOUDS-457: Add blobExists and blobMetadata (#47)
@demobox If you can find the time it would be great if you could review some other past commits too. Then I can open a PR fixing more of these nits. Thank you! :) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/47#issuecomment-50826899
Re: [jclouds-labs-aws] JCLOUDS-457: Add blobExists and blobMetadata (#47)
> @@ -120,7 +120,7 @@ public boolean createContainerInLocation(@Nullable
> Location location, String con
>
> @Override
> public boolean blobExists(String container, String key) {
> - throw new UnsupportedOperationException();
> + return this.blobMetadata(container, key) != null;
Again, if this is a jclouds-wide thing it also makes more sense to me changing
it, I agree with @demobox that we should be consistent.
Any thoughts about this @andrewgaul? I copied the style from s3, azureblob and
atmos in the past, and in many cases it wasn't right either.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/47/files#r15666115
Re: [jclouds-labs-aws] JCLOUDS-457: Add blobExists and blobMetadata (#47)
Thanks for the review @demobox! I can open a PR for these two changes or I can wait until we find something else to build a bigger refactor commit. Any thoughts? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/47#issuecomment-50808615
Re: [jclouds-labs-aws] JCLOUDS-457: Add blobExists and blobMetadata (#47)
> @@ -138,7 +138,13 @@ public String putBlob(String container, Blob blob,
> PutOptions options) {
>
> @Override
> public BlobMetadata blobMetadata(String container, String key) {
> - throw new UnsupportedOperationException();
> + PageSet blobs = this.list(container, null);
What about blobMetadataSet? as you pointed out in a previous commit, a PageSet
is not a list neither.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/47/files#r15664658
Re: [jclouds-labs-aws] JCLOUDS-457: Add blobExists and blobMetadata (#47)
> @@ -120,7 +120,7 @@ public boolean createContainerInLocation(@Nullable
> Location location, String con
>
> @Override
> public boolean blobExists(String container, String key) {
> - throw new UnsupportedOperationException();
> + return this.blobMetadata(container, key) != null;
It's ok with me, however I've been always told that the `this` modifier helps
identifying imported static methods from instance methods. Is there any reason
to remove it that I'm missing or is it just a matter of style?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-aws/pull/47/files#r15664571
Re: [jclouds-labs-aws] JCLOUDS-457: Documentation fixes (#51)
Thank you @demobox :) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs-aws/pull/51#issuecomment-50411176
