Add PrintInfo options to dump full keys, apply formatter

[drewfarris]

No description provided.

[ctubbsii]

@drewfarris Do these changes work on the current development branch? (master) Specifically, I'm concerned that Formatter is not public API, and adding it here could cause problems between branches.

Also, new features typically shouldn't be added to maintenance branches. I know we're preparing to release 1.10, so it would be a minor version, where new features are okay, but I still have reservations about going back to older versions to try to support new features there, but especially so if it stalls work on the latest development branch. It could also be very confusing to have the feature in 1.10, not have it in 2.0, and then to have it in 2.1. I could be convinced that these are important enough changes that we definitely need them in 1.10... but normally, I'd argue for changes like these to be put in the next version.

[drewfarris]

@drewfarris Do these changes work on the current development branch? (master) Specifically, I'm concerned that Formatter is not public API, and adding it here could cause problems between branches.

@ctubbsii Would Keith's suggestion of BiFunction<Key, Value, String> address this concern? It eliminates the reliance on the Formatter API entirely.

Also, new features typically shouldn't be added to maintenance branches. [..] . I could be convinced that these are important enough changes that we definitely need them in 1.10... but normally, I'd argue for changes like these to be put in the next version.

I agree with the principle of adding this to the newest branch. We really shouldn't be adding features to a 'legacy' baseline. I don't have a compelling argument for putting in in 1.10 other than it is the version we're using in our development and production environments currently and I have a practical need for this feature in that environment. I'd prefer not to maintain a patch/fork for this somewhat trivial feature that most folks won't observe.

I agree with your point about the fact that this should be targeted at all branches 1.10, 2.0, 2.1 - and I intend to do that based on your feedback. Unfortunately there's just not a nice way to do that without opening multiple PR's which I'd like to avoid until we all have notional agreement that this is a good thing to do (in order to maintain a single conversation thread.)

[drewfarris]

@ctubbsii - retargeted at master.

[keith-turner]

Would be nice to fail with an error message if --fullKeys and --formatter are specified.

[ctubbsii]

Unfortunately there's just not a nice way to do that without opening multiple PR's which I'd like to avoid until we all have notional agreement that this is a good thing to do (in order to maintain a single conversation thread.)

While I think new feature development in upstream Accumulo should focus on targeting the master branch, I understand the practical desire to have such features in currently used versions. Once we reach consensus on how this should look in master, I wouldn't be strongly opposed to backporting these flags to 1.10, if the change is small and can be done with minimal impact. I just think it's best to vet new features in master first, since that's where main development is happening.

[drewfarris]

Would be nice to fail with an error message if --fullKeys and --formatter are specified.

done. The wording is a little weird because both --fullKeys and --dump are mutually exclusive with --formater, but hopefully you'll agree this is sufficient as it.

If you assign the unchecked code to a variable, you can add this suppression on just that assignment, so it doesn't mask other (fixable, avoidable) problems in the method.

done. I'd wanted to do this, but hadn't figured out the variable assignment bit, thanks.

we should probably hard-fail by letting this exception fall through, rather than returning null and by effetively wasting time dumping a file in a format the user doesn't want.

done, also removed the null check as it was unnecessary.

[ctubbsii]

LGTM