Skip to content
This repository has been archived by the owner on Apr 22, 2022. It is now read-only.

Upgrade divolte and add default hdfs sink configuration.Fixes #4 #5

Merged
merged 2 commits into from Aug 16, 2018

Conversation

krisgeus
Copy link
Collaborator

No description provided.

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Small comments, but LGTM to me in general.

start.sh Outdated
if [ -z "$DIVOLTE_HDFS_SINK_WORKING_DIR" ]; then
# No specific dir set use default and create it if not exists
if [ ! -d /tmp/working ]; then
mkdir -p /tmp/working
Copy link
Contributor

Choose a reason for hiding this comment

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

mkdir -p does not really care if the directory already exists:

MacBook-Pro-van-Fokko:~ fokkodriesprong$ mkdir -p /tmp/
MacBook-Pro-van-Fokko:~ fokkodriesprong$ mkdir -p /tmp/
MacBook-Pro-van-Fokko:~ fokkodriesprong$ echo $?
0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thx. Done like suggested

start.sh Outdated
else
#make sure dir exists
if [ ! -d "$DIVOLTE_HDFS_SINK_WORKING_DIR" ]; then
mkdir -p "$DIVOLTE_HDFS_SINK_WORKING_DIR"
Copy link
Contributor

Choose a reason for hiding this comment

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

The mkdir -p "$DIVOLTE_HDFS_SINK_WORKING_DIR" might be nicer than the mkdir -p /tmp/working above.

I would change it to:

DIVOLTE_HDFS_SINK_WORKING_DIR=${DIVOLTE_HDFS_SINK_WORKING_DIR:/tmp/working}
mkdir -p "$DIVOLTE_HDFS_SINK_WORKING_DIR"

But this is a matter of taste I'd say.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like your taste. Done like this....

@Fokko Fokko merged commit 23014c6 into master Aug 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants