[MINOR] change new Exception to IllegalArgumentException

[adrian-wang]

No description provided.

[SparkQA]

Test build #33579 has finished for PR 6434 at commit be82259.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

I agree that it's not good practice to throw Exception, or RuntimeException, as these are generic base classes. sys.error just causes a RuntimeException though. I think that's not a real improvement and is less clear, and sys.error isn't used in the rest of the code as a rule anyway.

Instead these should be more meaningful exception types.

[rxin]

@srowen do you have a better suggestion for this case? In these cases, these cases are not supposed to happen at runtime. It is almost like an assert.

[srowen]

A subclass of RuntimeException is appropriate then. I use IllegalArgumentException for something basically related to input to the method, and IllegalStateException as a more general subclass of RuntimeException. It's quite minor, but while we're here, I'd rather do what I think is the general best practice in Java-land.

[rxin]

+1

[rxin]

Sorry @adrian-wang I think in this case IllegalArgumentException is more appropriate.

[SparkQA]

Test build #33706 has finished for PR 6434 at commit f8dea2d.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[rxin]

@adrian-wang can you update the exception to IllegalArgumentException so I can merge this?

[adrian-wang]

Oh, sorry for missing this, I'll update it very soon.

[SparkQA]

Test build #34428 has finished for PR 6434 at commit ee1b64f.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #34427 has finished for PR 6434 at commit f7c53e9.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[rxin]

Thanks. I've merged this.