Skip to content

[Flink 5404] Consolidate and update S3 documentation#3054

Closed
medale wants to merge 4 commits intoapache:masterfrom
medale:FLINK-5404
Closed

[Flink 5404] Consolidate and update S3 documentation#3054
medale wants to merge 4 commits intoapache:masterfrom
medale:FLINK-5404

Conversation

@medale
Copy link
Contributor

@medale medale commented Jan 3, 2017

Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the How To Contribute guide.
In addition to going through the list, please provide a meaningful description of your changes.

  • General

    • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
    • The pull request addresses only one issue
    • Each commit in the PR has a meaningful commit message (including the JIRA id)
  • Documentation

    • Documentation has been added for new functionality
    • Old documentation affected by the pull request has been updated
    • JavaDoc for public methods has been added
  • Tests & Build

    • Functionality added by the pull request is covered by tests
    • mvn clean verify has been executed successfully locally or a Travis build has passed

@medale
Copy link
Contributor Author

medale commented Jan 4, 2017

Error about RocksDB caused by: java.lang.UnsatisfiedLinkError: Native Library /tmp/librocksdbjni-linux64.so already loaded in another classloader at java.lang.ClassLoader.loadLibrary1(ClassLoader.java:1931). Since I only changed documentation this seems like an unrelated error with the overall build. How do I continue or re-run?

Copy link
Contributor

@tzulitai tzulitai left a comment

Choose a reason for hiding this comment

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

LGTM, I like the changes. Just some minor styling comments ;)
+1 to merge once those are addressed.

- the `flink-conf.yaml` has set the `fs.hdfs.hadoopconf` property set to the Hadoop configuration directory.
- the Hadoop configuration (in that directory) has an entry for the required file system. Examples for S3 and Alluxio are shown below.
- the required classes for using the file system are available in the `lib/` folder of the Flink installation (on all machines running Flink). If putting the files into the directory is not possible, Flink is also respecting the `HADOOP_CLASSPATH` environment variable to add Hadoop jar files to the classpath.
- the `flink-conf.yaml` has set the `fs.hdfs.hadoopconf` property to the Hadoop configuration directory. For automated testing or running from an IDE the directory containing `flink-conf.yaml` can be set by defining the FLINK_CONF_DIR environment variable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add ` around FLINK_CONF_DIR ==> FLINK_CONF_DIR

```

This registers `path/to/etc/hadoop` as Hadoop's configuration directory with Flink.
This registers `/path/to/etc/hadoop` as Hadoop's configuration directory with Flink. Flink will look for the "core-site.xml" and "hdfs-site.xml" files in the specified directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

Reference to core-site.xml and hdfs-site.xml here uses double apostrophes, while ` is used in other places.
Lets keep that styling consistent here :)

@tzulitai
Copy link
Contributor

tzulitai commented Jan 5, 2017

@medale The test failure is unrelated, it's ok to ignore them. The tests will also re-run once Travis detects more commits are pushed to the branch.

@medale
Copy link
Contributor Author

medale commented Jan 6, 2017

@tzulitai thank you for the feedback and catching those stylistic elements! I made the changes and recommitted.

@uce
Copy link
Contributor

uce commented Jan 24, 2017

Thanks for the PR. Looks good to me. Merging this...

@asfgit asfgit closed this in f0d96e3 Jan 24, 2017
joseprupi pushed a commit to joseprupi/flink that referenced this pull request Feb 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants