Add archive type option #494
Conversation
|
cc @pickypg |
| import com.elastic.support.util.ResourceCache; | ||
| import com.elastic.support.util.SystemProperties; | ||
| import com.elastic.support.util.SystemUtils; | ||
| import com.elastic.support.util.*; |
There was a problem hiding this comment.
Not a fan of * imports, but it's fine for this since the code already has others.
There was a problem hiding this comment.
Done.
It's weird, IntelliJ keeps adding them back automatically.
There was a problem hiding this comment.
IntelliJ has pretty low defaults for doing that. Most teams I work on suggest setting the limit to 9999.
| @@ -132,8 +132,7 @@ public boolean runInteractive() { | |||
| return Collections.singletonList("Invalid Date or Time format. Please enter the date in format YYYY-MM-dd HH:mm"); | |||
| } | |||
|
|
|||
| return emptyList; | |||
|
|
|||
| return null; | |||
There was a problem hiding this comment.
Careful with returning null instead of an empty list. Some parts of the code expect things that they cannot safely expect.
Given that this code relates explicitly to fetching stack monitoring data, I am unsure if you tested this behavior?
There was a problem hiding this comment.
Done.
This PR also fixes a nasty bug. When port is invalid, many port validation errors are printed out. This was happening because emptyList is mutable and shared. I have another PR in the queue to prevent those issues in the future.
There was a problem hiding this comment.
@pickypg actually, caller does expect it to return null and I changed it back in cd1a275.
Here is where it gets called:
When validateTimeWindow() returns null it defaults to emptyList. When it returns an empty list, it also adds an empty list to errors.
In the end, it doesn't really matter if validateTimeWindow() returns a empty list or null.
I will fix those issues in my next PR.
Part of #485.
Right, the utility tries to zip the output dir, if zip fails, it fallbacks to tar.gz.
However, this should be deterministic, devs should be able to choose which output file type they want: zip or tar.gz.