Refactor duplicated code in SubmitterMain and SchedulerConfig

[mycFelix]

As #1597 mentioned, the duplicated code is found in SubmitterMain and SchedulerConfig.

This PR is trying to fix it and the changes are following:

• add an new method String getPkgType(String topologyBinaryFile) in FileUtils
• invoke getPkgType method in SubmitterMain
• invoke getPkgType method in SchedulerConfig

[objmagic]

👍

[billonahill]

Thanks for the removal of dup code!

Could you also please change to not abbreviate pkg (e.g., packageType and getPackageType).

Also instead of returning strings, better to use an enum, maybe as an inner enum in FileUtils:

enum PackageType {
  PEX,
  JAR,
  TAR
}

[objmagic]

sorry for merging this too fast. I'll do this.

[objmagic]

actually, returning enum might not be what we want, since we need to put value as string into config. See: https://github.com/twitter/heron/pull/1599/files#diff-1043de1ec8f8e4c0194d4e6035e654cfL80

[billonahill]

Config.put takes an object as it's value, so we could put enum values in and add a getter that returns it. We did this with ByteAmount for example:

https://github.com/twitter/heron/blob/master/heron/spi/src/java/com/twitter/heron/spi/common/Config.java#L79

Then we refactor all consumers to use the enum.

The only catch is that if we add the helper to Config, then the enum needs to live somewhere central like in spi. This might not be a bad thing though. It's much better to have a strong set of package types than to pass strings around...

[objmagic]

where do you think is appropriate to put this? I am really bad at finding an appropriate class to add code😔

[mycFelix]

@objmagic - hi, Is there anything I could do? The method name which @billonahill mentioned is better . However, I don't think adding an new enum class is a good idea. Perhaps we could add new static constants in FileUtils instead of handling string variable explicitly, for example

private static final String PEX = "pex";

[objmagic]

@mycFelix any reason why using enum is not a good idea?

[mycFelix]

@objmagic - Cause String pkgType is used for Config's value. Just in my opion, this scene is applicable to the principle of Occam's razor -- If not necessary, do not increase the entity

[billonahill]

Config supports objects and we should favor type safety where ever possible. Doing so will make code easier to maintain and less prone to bugs than by passing string values around.

[objmagic]

@mycFelix imo Bill has better arguments. I will submit a PR to use enum.