Skip to content
This repository has been archived by the owner on May 14, 2021. It is now read-only.

MINIFI-455: Updating C2 readme to include S3. Simplifying minifi-c2-c… #126

Closed
wants to merge 1 commit into from
Closed

Conversation

jzonthemtn
Copy link
Contributor

MINIFI-455: Updating C2 readme to include S3. Simplifying minifi-c2-context.xml.

Thank you for submitting a contribution to Apache NiFi - MiNiFi.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

  • Does your PR title start with MINIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

  • Has your PR been rebased against the latest commit within the target branch (typically master)?

  • Is your initial contribution a single, squashed commit?

For code changes:

  • Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi-minifi folder?
  • Have you written or updated unit tests to verify your changes?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file, including the main LICENSE file under minifi-assembly?
  • If applicable, have you updated the NOTICE file, including the main NOTICE file found under minifi-assembly?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

@Paul-Verardi
Copy link

Paul-Verardi commented May 14, 2018

Hi Jeff, I am struggling to get this working and I believe my issue involves the minifi-c2-context.xml values that I am attempting to pass.

My S3 Folder structure inside of my bucket is as follows:
TenantConfigs/tenanta/config.text.yml.v1

If I try to pass "TenantConfigs/" as the prefix and "${class}/config" as the pathstring, I get the following error:

o.a.nifi.minifi.c2.service.ConfigService Error reading or checksumming configuration file
java.io.IOException: Attempted read on closed stream.

May 14, 2018 12:42:19 PM com.sun.jersey.spi.container.ContainerResponse logException
SEVERE: Mapped exception to response: 500 (Internal Server Error)
javax.ws.rs.WebApplicationException
at org.apache.nifi.minifi.c2.service.ConfigService.getConfig(ConfigService.java:231)

Last week, when we were trying to get MiNiFI Agent to connect to our c2 using 0.4 version, we had to remove the javax.ws.rs-api-2.1.jar from both c2 and agent in order for it to connect. Now it seems this jar is needed for this implementation with s3? Has this been tested on version 4 yet?

@jzonthemtn
Copy link
Contributor Author

Hi @Paul-Verardi, nothing comes to mind right off but I will walkthrough it again to see. Thanks!

@jzonthemtn
Copy link
Contributor Author

jzonthemtn commented May 14, 2018

@Paul-Verardi, like you I also had to remove the javax.ws.rs-api-2.1.jar or I would get an exception around the UriBuilder class. That will need looked into more (see issue). But for now, I configured S3 in my minifi-c2-context.xml as (in constructor argument order):

  • My bucket name, bucket-name.
  • Empty value for prefix.
  • Path pattern is ${class}/config.
  • Set an access/secret key since I wasn't testing on AWS.
  • Left region as us-east-1.

My config file in S3 is stored at:

s3://bucket-name/raspi3/config.text.yml.v1

I allowed anonymous access to C2, started C2 and did a GET to C2:

curl http://localhost:10080/c2/config?class=raspi3&version=1 -H "Accept: text/yml"

The response was the contents of the config file from S3.

I did this using this branch so it's MiNiFI 0.4.0 with a few changes. I hope this is helpful. You might try the NiFi users mailing list if you haven't already in case someone else has run into the same problem.

@Paul-Verardi
Copy link

@jzonthemtn Thanks for looking into this, I will try to get it working under the same implementation you described.

@apiri
Copy link
Member

apiri commented May 22, 2018

Sorry for overlooking this PR. These changes look good. Will merge them in. Thank you!

@asfgit asfgit closed this in 9fc758b May 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants