magpierre commented on code in PR #82:
URL: https://github.com/apache/spark-connect-go/pull/82#discussion_r1812406023
##########
spark/client/conf.go:
##########
@@ -0,0 +1,127 @@
+package client
+
+import (
+ "context"
+
+ proto "github.com/apache/spark-connect-go/v35/internal/generated"
+ "github.com/apache/spark-connect-go/v35/spark/client/base"
+)
+
+// Public interface RuntimeConfig
+type RuntimeConfig interface {
Review Comment:
I have followed:
https://github.com/apache/spark/blob/master/python/pyspark/sql/connect/conf.py
since it is identical to what we doing since it is implementing spark-connect
proto calls. If you look at the proto mplementation, the proto allows for
arrays for all parameters whilst the interface dictates a single entry access.
I am happy to adjust the existing ones to follow the existing interface
specification to the letter, but I do believe there is power here that we
ideally would like to expose to the user and I think it is really not that much
harder to write []string{"key"}, rather than "key" but I get what you are after
. Would it be ok to expose additional methods so you could still do the array
approach? It simplifies fetching all keys from get all and then check which is
modifiable and so on. Furthermore, all types are translated to string for proto
so from a coding standpoint I think it is best to have an interface that is
clean and exposes the actually used type and let
the user do the type conversion, rather than hide it in the function. But if
the decision is to be 1-1 with the python functions, I am fine with that and
adjust as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
