[LIVY-654] Ability to validate string before creating state/kind of session/statement

[o-shevchenko]

What changes were proposed in this pull request?

https://issues.apache.org/jira/browse/LIVY-654

Kind.scala, SessionState.scala and StatementState.java don't include a method for string validation. It might be very useful to verify if some string can be converted to an enum type.

For example, to parse JSON string we need to write something like this (to prevent IllegalStateException):

Try {
  value match {
  case JsString(state) => SessionState(state)
  case _ => throw MyException
  }
} 
match {
 case Success(state) => state
 case Failure(ex) => throw MyException
}

it can be simplified to:

value match {
  case JsString(state) if SessionState.isValid(state) => SessionState(state)
  case _ => throw MyException
}

The second problem is StatementState.java enum use Pascal case instead of upper case
https://github.com/apache/incubator-livy/blob/412ccc8fcf96854fedbe76af8e5a6fec2c542d25/rsc/src/main/java/org/apache/livy/rsc/driver/StatementState.java#L26

For example, Waiting instead of WAITING. This isn't fit Java code style.

In this case, we can't use StatementState.valueOf("waiting".toUpperCase()) as usually but

StatementState.valueOf(state.toLowerCase().capitalize). Not sure if we can change it now.

How was this patch tested?

Existed tests

[o-shevchenko]

Can we also move object SQL and other kinds inside object Kind to be able to use:
import org.apache.livy.sessions.Kind._ instead of import org.apache.livy.sessions.{Kind, PySpark, SessionState, Spark, SparkR, SQL}
It looks more convenient.
Is it critical for backward compatibility?

[o-shevchenko]

Is better to use values everywhere (instead of kinds/states)?

[o-shevchenko]

Can we change naming (upper case instead of Pascal case) for enum values. For example, WAITING instead of Waiting. This doesn't fit Java code style.
Also, in this case, we can't use StatementState.valueOf("waiting".toUpperCase()) as usually but need to capitalize first letter

StatementState.valueOf(state.toLowerCase().capitalize).

[jerryshao]

What's purpose of changes here to switch to case class?

[o-shevchenko]

To unify access to enumeration type and to be able to create sequence of all states

val states: Seq[SessionState] = Seq(NotStarted(), Starting(), Recovering(), Idle(), Running(),
    Busy(), ShuttingDown(), Killed(), Error(), Dead(), Success())

[jerryshao]

The changes here will create many small SessionState objects, which is unnecessary and can be replaced by singleton.

[o-shevchenko]

we need to use either object or class to be able to create a sequence of states. as far as I see, you changed case class to object for FinishedSessionState classes to fix the problem with time variable initialization, so we can't use singleton everywhere. therefore, I think this is not critical to use case class instead of object for other states.

[jerryshao]

Where did this get called?

[o-shevchenko]

this is public API to use as I described in the description

[jerryshao]

Then how do users use this public api?

[o-shevchenko]

to verify that we can create StateSession/Kind object from a string without additional logic for catching exceptions.
There is an example in the description.

[o-shevchenko]

you need to add livy-core dependency in your project if you are asking about it

[jerryshao]

Why do you need to create StateSession or Kind object?

[o-shevchenko]

to simply use it in the code instead of hardcoding string values like "idle", sometimes I need to check session state or wait for state, for example, create a session and wait for status "idle" to be able to run statement. I don't want to use string variables everywhere. I need an enumeration. I can create my own in my code which totally duplicates Livy enum but better to use it from Livy. it is the typical case, isn't it?

[jerryshao]

Yes, that's the case. But from my point I never expect to expose this to users

[o-shevchenko]

hmm, having a method to check if the object is a part of the enum is common practice for Scala, it can simplify using of your enum to don't force user to catch exception or write own method to check if object exists in enum values (if exist, we don't have it now in Kind.scala, SessionState.scala that's why I have added sequences of values in these classes)

[github-actions]

This pull request has been automatically marked as stale because it has had no activity for at least 3 months. If you are still working on this change or plan to move it forward, please leave a comment or push a new commit so we know to keep it open. Otherwise, this PR will be closed automatically in about one month. Thank you for your contribution to Apache Livy!