You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by reptillicus <no...@github.com> on 2018/11/01 22:05:27 UTC

[jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

as discussed in this mail thread: https://lists.apache.org/thread.html/e10af185b92545935223cd070a9eabbda2898ba7265a223512c07f3f@%3Cuser.jclouds.apache.org%3E

Also added some tests in BaseContainerIntegrationTest that test clearContainer with prefix etc. 
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/1254

-- Commit Summary --

  * Removed check for prefix in clearContainer, as discussed in this mail thread: https://lists.apache.org/thread.html/e10af185b92545935223cd070a9eabbda2898ba7265a223512c07f3f@%3Cuser.jclouds.apache.org%3E

-- File Changes --

    M blobstore/src/main/java/org/jclouds/blobstore/strategy/internal/DeleteAllKeysInList.java (3)
    M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseBlobStoreIntegrationTest.java (16)
    M blobstore/src/test/java/org/jclouds/blobstore/integration/internal/BaseContainerIntegrationTest.java (59)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/1254.patch
https://github.com/jclouds/jclouds/pull/1254.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by reptillicus <no...@github.com>.
Howdy from texas and thanks for merging that! If there is something else that is needed in BlobStore I'd be happy to take a look. 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-435621925

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by reptillicus <no...@github.com>.
So is the expected behavior of the FileSystem blob store to also remove any
empty folders if a path is cleared and no files exist in there? As in, same
behavior as S3 type blob store.

On Fri, Nov 16, 2018 at 12:36 PM joe meiring <jo...@gmail.com>
wrote:

> Sure, I'll poke around this afternoon.
>
> On Wed, Nov 14, 2018 at 4:53 PM Andrew Gaul <no...@github.com>
> wrote:
>
>> Unfortunately filesystem has its own logic so I added a check to disable
>> this in 1ae735bb7d8fa4d512e744f79582477efdfd7f26 and explained the problem
>> in a comment. @reptillicus <https://github.com/reptillicus> could you
>> investigate this? You can run the single test with
>>
>> mvn integration-test -am -pl :filesystem -Plive -Dtest=FilesystemContainerIntegrationTest#testClearWithOptions -DfailIfNoTests=false
>>
>> —
>> You are receiving this because you were mentioned.
>> Reply to this email directly, view it on GitHub
>> <https://github.com/jclouds/jclouds/pull/1254#issuecomment-438849626>,
>> or mute the thread
>> <https://github.com/notifications/unsubscribe-auth/ABXuOhPzJZmoLc2FHl0MQqPxC4PZlRqrks5uvJ8EgaJpZM4YJqLZ>
>> .
>>
>


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-439502221

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by Andrew Gaul <no...@github.com>.
filesystem should act like other blobstores to the most practical extent possible.  It does this correctly today if you have one blob named `a/b` and remove it.  Both the file `a/b` and the enclosing directory are removed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-439596044

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by Andrew Gaul <no...@github.com>.
Closed #1254.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#event-1944542954

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by Andrew Gaul <no...@github.com>.
Fixed nits, reworded commit message, and pushed to master as 22ce5484a412bc06ef62995675c07e7a85f66bdf and 2.1.x as 9ede9d3ddc9ac4aa175bf9c364b83a161bc93294.  Thank you for your contribution @reptillicus!  Also hello to my alma mater.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-435619003

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by reptillicus <no...@github.com>.
Sure, I'll poke around this afternoon.

On Wed, Nov 14, 2018 at 4:53 PM Andrew Gaul <no...@github.com>
wrote:

> Unfortunately filesystem has its own logic so I added a check to disable
> this in 1ae735bb7d8fa4d512e744f79582477efdfd7f26 and explained the problem
> in a comment. @reptillicus <https://github.com/reptillicus> could you
> investigate this? You can run the single test with
>
> mvn integration-test -am -pl :filesystem -Plive -Dtest=FilesystemContainerIntegrationTest#testClearWithOptions -DfailIfNoTests=false
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/jclouds/jclouds/pull/1254#issuecomment-438849626>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/ABXuOhPzJZmoLc2FHl0MQqPxC4PZlRqrks5uvJ8EgaJpZM4YJqLZ>
> .
>


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-439486044

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by Andrew Gaul <no...@github.com>.
We definitely appreciate the help.  The list of open blobstore issues has grown quite large:

https://issues.apache.org/jira/browse/JCLOUDS-1465?jql=project%20%3D%20JCLOUDS%20AND%20component%20%3D%20jclouds-blobstore%20AND%20status%20%3D%20open

Please jump in if something interests you!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-435622811

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by Andrew Gaul <no...@github.com>.
Unfortunately filesystem has its own logic so I added a check to disable this in 1ae735bb7d8fa4d512e744f79582477efdfd7f26 and explained the problem in a comment.  @reptillicus could you investigate this?  You can run the single test with 

```
mvn integration-test -am -pl :filesystem -Plive -Dtest=FilesystemContainerIntegrationTest#testClearWithOptions -DfailIfNoTests=false
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-438849626

Re: [jclouds/jclouds] Removed check for prefix/delimiter in clearContainer (#1254)

Posted by Andrew Gaul <no...@github.com>.
@timuralp you probably added this check; could you look at this pull request?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1254#issuecomment-435216716