Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closes #298 - Enable MiniFluo administration from fluo admin script #318

Merged
merged 1 commit into from
Nov 12, 2014

Conversation

mikewalch
Copy link
Member

Pull request for #298

@keith-turner
Copy link
Contributor

Should we rework phrasecount and quickstart examples to use this new feature? Thats one way of testing this. If we did that, maybe we could put the work in branch in phrasecount and quickstart repos for now. Leaving master on phrasecount and quickstart based against alpha until beta is released.

```
Optionally, check out a stable tag (if you don't want to build a release from master):
```
git checkout 1.0.0-alpha-1
Copy link
Contributor

Choose a reason for hiding this comment

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

Could go ahead and put 1.0.0-beta-1 here, as these instructions will not make sense for alpha

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in f09fbd4

@keith-turner
Copy link
Contributor

Should have an IT that exercises io.fluo.mini.start.accumulo. I was thinking that TestMiniBase could be modified for this, but took a look at it and decided against this. Many test could extend TestMiniBase and it probably best if they all use the shared mini accumulo. Would be better to have a single IT that starts its own minifluo which starts an accumulo.

</excludes>
</dependencySet>
<dependencySet>
<outputDirectory>lib/log4j</outputDirectory>
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting... maybe we could eventually have a lib/hadoop and lib/accumulo

log.info("Started MiniAccumulo(accumulo="+cluster.getInstanceName()+" zk="+cluster.getZooKeepers()+")");

// configuration that must overridden
config.setAccumuloInstance(cluster.getInstanceName());
Copy link
Contributor

Choose a reason for hiding this comment

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

should an exception be thrown if these accumulo props are already set? I am not 100% sure about this. I can think of situation where it would be annoying.... but I can also think of situations where mini fluo does not use the accumulo instance I expected.

Copy link
Member Author

Choose a reason for hiding this comment

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

In 03e35f5, We will now throw an exception. An exception is not thrown although if a client property is uncommented but not set (as fluo.properties defaults to this).

@mikewalch
Copy link
Member Author

I created a test that exercises MiniFluo (when it starts its own accumulo) in 1290086

@keith-turner
Copy link
Contributor

+1 (would be really nice to make MiniIT use target, but other than that it looks good)

mikewalch added a commit that referenced this pull request Nov 12, 2014
Closes #298 - Enable MiniFluo administration from fluo admin script
@mikewalch mikewalch merged commit 4edaa61 into apache:master Nov 12, 2014
@mikewalch mikewalch deleted the fluo-298 branch November 12, 2014 21:16
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.

None yet

2 participants