FluoConfiguration should derive table name from application name if not set

[ctubbsii]

FluoConfiguration requires the application name to be set. However, it does not require the table name to be set (defaults to "fluo"). Problems can occur if separate applications use the same table. The table name should be derived from the application name, if not explicitly set. Or, the table name should be required.

Additionally, the configuration should have more javadocs explaining what the application name is used for, and how it relates to the table name, and explaining the pitfalls of using the same table for different applications.

[ctubbsii]

If table name was required, the application name could be automatically derived from the table name.

[kennethmcfarland]

I think this was handled but would like to confirm. I think basically it was made so the table name is required. This is the call chain that makes me believe so:

fluo/modules/api/src/main/java/org/apache/fluo/api/config/FluoConfiguration.java

Line 264 in 01fc804

getAccumuloTable();

calls this

fluo/modules/api/src/main/java/org/apache/fluo/api/config/FluoConfiguration.java

Lines 551 to 553 in 01fc804

	public String getAccumuloTable() {
	return getDepNonEmptyString(ACCUMULO_TABLE_PROP, ADMIN_ACCUMULO_TABLE_PROP);
	}

calls this

fluo/modules/api/src/main/java/org/apache/fluo/api/config/FluoConfiguration.java

Lines 1196 to 1204 in 01fc804

	private String getDepNonEmptyString(String property, String depProperty) {
	if (containsKey(property)) {
	return getNonEmptyString(property);
	} else if (containsKey(depProperty)) {
	return getNonEmptyString(depProperty);
	} else {
	throw new NoSuchElementException(property + " is not set!");
	}
	}

calls this

fluo/modules/api/src/main/java/org/apache/fluo/api/config/FluoConfiguration.java

Lines 1184 to 1189 in 01fc804

	private String getNonEmptyString(String property) {
	String value = getString(property);
	Objects.requireNonNull(value, property + " cannot be null");
	Preconditions.checkArgument(!value.isEmpty(), property + " cannot be empty");
	return value;
	}

This validation is also there:

fluo/modules/api/src/main/java/org/apache/fluo/api/config/FluoConfiguration.java

Lines 1030 to 1035 in 01fc804

	public boolean hasRequiredAdminProps() {
	boolean valid = true;
	valid &= hasRequiredClientProps();
	valid &= verifyStringPropSet(ACCUMULO_TABLE_PROP, ADMIN_ACCUMULO_TABLE_PROP);
	return valid;
	}

Hope this is helpful, thanks @ctubbsii !

[ctubbsii]

@kpm1985 I think you're right. It looks like either "ACCUMULO_TABLE_PROP" or "ADMIN_ACCUMULO_TABLE_PROP" must be set. There is no default. It will use one of those two only. However, I can't find the commit which changed this behavior, and it looks like we still have some code which assumes it could be unset:

fluo/modules/mini/src/main/java/org/apache/fluo/mini/MiniFluoImpl.java

Lines 123 to 127 in 01fc804

	// configuration that only needs to be set if not by user
	if ((config.containsKey(FluoConfiguration.ACCUMULO_TABLE_PROP) == false)
	|| config.getAccumuloTable().trim().isEmpty()) {
	config.setAccumuloTable("fluo");
	}