Skip to content
This repository was archived by the owner on Jul 9, 2022. It is now read-only.
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions azure-storage/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-cloud-stream-azure-storage-parent</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

azure-storage module must to be included in the root pom. Otherwise it isn't imported into IDE automatically.
And I guess won't be visible for building via Maven.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in latest commit.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you are doing locally, but this change isn't present in the current PR state.
You should do changes, perform commit and push to origin.
The PR here in GH picks up all the changes automatically.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if you were suggesting the dependency be moved to the parent or something else. The former is what is reflected below.

@artembilan artembilan Aug 22, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Looks like we don't understand each other a bit.
Sorry for my bad English or for anything else I'm saying is unclear.
I mean that we should add <module>azure-storage</module> into <modules> of root pom of the entire project.

I could make the polishing for you do demonstrate what I have in mind on the matter, but I can't do that against your master. Having my infrastructure polishing you can integrate that commit into your branch and go ahead with other Blob Storage stuff to reach some finish.

But that would be enough complex if you you are still on master.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the first part. This should have been obvious to me - thank you for clarifying? :-)

<packaging>pom</packaging>

<properties>
<azure.version>4.3.0</azure.version>
</properties>

<parent>
<groupId>org.springframework.cloud.stream.app</groupId>
<artifactId>spring-cloud-stream-app-starters</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shoud use starter-parent as the parent, at least at the time of writing

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conflicts with how the aws-s3 parent is defined - I used this as a template. Please advise, as I get build errors trying to make it starter-parent

<version>1.0.0.BUILD-SNAPSHOT</version>
</parent>

<modules>
<module>spring-cloud-starter-stream-sink-azure-blob</module>
</modules>

<dependencies>
<dependency>
<groupId>com.microsoft.azure</groupId>
<artifactId>azure-storage</artifactId>
<version>${azure.version}</version>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# spring-cloud-stream-wasb-sink
#### A Windows Azure Storage Blob sink module for Spring Cloud Stream

This module is an MVP implementation of a BlockBlob-based sink module
for text payloads originating from a Spring Cloud Dataflow stream.

An example stream definition:

```
dataflow:> app register --type sink --name wasb --uri file:///Users/kdunn/.m2/repository/io/pivotal/pde/wasb-sink/0.0.1-SNAPSHOT/wasb-sink-0.0.1-SNAPSHOT.jar

# CloudBlockBlob (every payload overwrites)
dataflow:> stream create --name testWasb --definition 'time | wasb --accountName="scdftest" --accountKey="<YOUR STORAGE ACCOUNT KEY HERE>" --containerName="time" --blobName="test" ' --deploy

# CloudAppendBlob (every payload appends)
dataflow:> stream create --name testWasb --definition 'time | wasb --accountName="scdftest" --accountKey="<YOUR STORAGE ACCOUNT KEY HERE>" --containerName="time" --blobName="testAppend" --appendOnly=true --overwriteExistingAppend=true ' --deploy
```
47 changes: 47 additions & 0 deletions azure-storage/spring-cloud-starter-stream-sink-azure-blob/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-cloud-starter-stream-sink-wasb</artifactId>
<name>spring-cloud-starter-stream-sink-wasb</name>
<description>Spring Cloud Stream Windows Azure Blob Storage Sink</description>

<parent>
<groupId>org.springframework.cloud.stream.app</groupId>
<artifactId>spring-cloud-stream-azure-storage-parent</artifactId>
<version>1.0.0.BUILD-SNAPSHOT</version>
</parent>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<java.version>1.8</java.version>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not that we have moved to Java 8 already here...
I assume that we fully rely on the Spring Boot options from the top-level parent.

</properties>

<dependencies>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-stream-kafka</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need this.
More over any SCSt-apps shouldn't know anything about binders or dataflow.

</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependency should be changed to this:

        <!-- Test Scope -->
        <dependency>
            <groupId>org.springframework.cloud.stream.app</groupId>
            <artifactId>app-starters-test-support</artifactId>
        </dependency>

<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this?
Isn't it a transitive dependency of the azure-storage if that ?

<version>3.0</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let't get rid of this one and rely just on the org.apache.commons.logging.Log, which is transitive from Spring Core.

</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-configuration-processor</artifactId>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this one?
I think that one from the root pom should be enough.

<optional>true</optional>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* Copyright 2016 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.springframework.cloud.stream.app.azure.storage.sink;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.cloud.stream.annotation.EnableBinding;
import org.springframework.cloud.stream.messaging.Sink;
import org.springframework.context.annotation.DependsOn;
import org.springframework.integration.annotation.ServiceActivator;
import org.springframework.messaging.Message;
import org.springframework.messaging.MessagingException;

// Include the following imports to use blob APIs.
import com.microsoft.azure.storage.CloudStorageAccount;
import com.microsoft.azure.storage.blob.BlobContainerPermissions;
import com.microsoft.azure.storage.blob.BlobContainerPublicAccessType;
import com.microsoft.azure.storage.blob.CloudBlob;
import com.microsoft.azure.storage.blob.CloudAppendBlob;
import com.microsoft.azure.storage.blob.CloudBlobClient;
import com.microsoft.azure.storage.blob.CloudBlobContainer;
import com.microsoft.azure.storage.blob.CloudBlockBlob;


/**
* @author Kyle Dunn
*/
@EnableBinding(Sink.class)
@EnableConfigurationProperties(AzureBlobSinkProperties.class)
@SpringBootApplication
public class AzureBlobSinkConfiguration {

protected static Logger LOG = LoggerFactory.getLogger(AzureBlobSinkConfiguration.class);

@Autowired
private AzureBlobSinkProperties properties;

private CloudBlob blobService;

@Autowired
public void setBlobService() {
// Define the connection-string with your values
final String storageConnectionString =
"DefaultEndpointsProtocol=" + this.properties.getDefaultEndpointsProtocol() +
";AccountName=" + this.properties.getAccountName() +
";AccountKey=" + this.properties.getAccountKey();

try {
// Setup the cloud storage account.
CloudStorageAccount account = CloudStorageAccount.parse(storageConnectionString);

LOG.info("getBlobService() : using account {}", this.properties.getAccountName());

// Create a blob service client
CloudBlobClient blobClient = account.createCloudBlobClient();

// Get a reference to a container
// The container name must be lower case
CloudBlobContainer container = blobClient.getContainerReference(this.properties.getContainerName().toLowerCase());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A blob container can be as an option of incoming message, e.g. some header.
Therefore any reference to CloudBlobContainer object should be done during @ServiceActivator process.

And in general it is would be much better do not do any hard network processes during bean initialization phase.
For this purpose we have SmartLifecycle contract with its autoStartup = true.

Looks like CloudStorageAccount and CloudBlobClient instantiations are safe of network operations. Therefore we still can do that during @PostConstruct phase. Although, as I said before, it would be better to move the to separate @Configuration

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to defer on these changes as I am following your suggestions but the implementation of them isn't quite as clear. You can follow up on the new PR against a different branch once this one is closed.


LOG.info("getBlobService() : using container {}", this.properties.getContainerName());

if (this.properties.getAutoCreateContainer()) {
container.createIfNotExists();
}

// Make the container public
if (this.properties.getPublicPermission()) {
LOG.info("getBlobService() : making container publically accessible");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: publicly.

But I don't see reason in this log message...


// Create a permissions object
BlobContainerPermissions containerPermissions = new BlobContainerPermissions();

// Include public access in the permissions object
containerPermissions.setPublicAccess(BlobContainerPublicAccessType.CONTAINER);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// Set the permissions on the container
container.uploadPermissions(containerPermissions);
}

LOG.info("getBlobService() : using blob name {}", this.properties.getBlobName());

if (this.properties.getAppendOnly()) {
this.blobService = container.getAppendBlobReference(this.properties.getBlobName());
if (this.properties.getOverwiteExistingAppend()) {
((CloudAppendBlob) blobService).createOrReplace();
}
}
else {
this.blobService = container.getBlockBlobReference(this.properties.getBlobName());
}
} catch (Exception e) {
// Log the stack trace.
LOG.error("getBlobService() : {}", e.getMessage());
//throw e;
}
}

@ServiceActivator(inputChannel=Sink.INPUT)
public void pushToAzureBlob(Message<?> message) throws MessagingException {
try {
// Upload the payload to the blob
if (this.properties.getAppendOnly()) {
((CloudAppendBlob) blobService).appendText(message.getPayload().toString() + "\n");
}
else {
((CloudBlockBlob) blobService).uploadText(message.getPayload().toString());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure in this useless toString(). Would be better to expect (and require) byte[]. At least any object can be converted to byte[], but not everything toString() is so good.

}

} catch (Exception e) {
// Log the stack trace.
LOG.error("pushToAzureBlob() : {}", e.getMessage());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution doesn't look good.
Would be better just don't catch any exceptions and let them bubble to the caller.

}

}

public static void main(String[] args) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this stuff. Would be better to write your own @SpringBootApplication with @Import for this @Configuration.

SpringApplication.run(AzureBlobSinkConfiguration.class, args);
}

}
Loading