[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[ https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16258300#comment-16258300 ] ASF GitHub Bot commented on S2GRAPH-170: Github user asfgit closed the pull request at: https://github.com/apache/incubator-s2graph/pull/127 > Create Interface for S2Edge/S2Vertex/S2Graph. > - > > Key: S2GRAPH-170 > URL: https://issues.apache.org/jira/browse/S2GRAPH-170 > Project: S2Graph > Issue Type: Improvement >Affects Versions: 0.2.1 >Reporter: DOYUNG YOON >Assignee: DOYUNG YOON > Fix For: 0.2.1 > > Original Estimate: 504h > Remaining Estimate: 504h > > h2. Problem Statement > S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class. > Even though lots of code touch theses two class, there is no interface > defined currently. This means lots of code is interact with these class in > different way all by their own way, and this make extremely hard to make any > change on these two classes. > For example, I was working on S2GRAPH-80 to provide java client, and there > are too many places to be changed since all theses places use concrete > implementation class S2Edge/S2Vertex/S2Graph, not the interfaces. Not just > for S2GRAPH-80, but any other issues that need to change theses classes would > benefit by communicating through interface. > h2. Suggestion > Define interface and change code base to communicate through theses > interfaces. > # Create interface S2Edge/S2Vertex/S2Graph that implement Tinkerpop’s > Edge/Vertex/Graph interface > # Extract tinkerpop interface related implementations > # Change caller of S2Edge/S2Vertex/S2Graph to call interface rather than > concrete implementation. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[ https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16257897#comment-16257897 ] ASF GitHub Bot commented on S2GRAPH-170: Github user daewon commented on the issue: https://github.com/apache/incubator-s2graph/pull/127 It looks good to solve problems that can occur with Java and Scala using interfaces like `S2[Edge, Vertex, Graph] Like`. Also, providing the `CompletableFuture` that can be used as a standard interface in Java is very convenient for users as below. `` ` def getEdgesJava (q: Query): CompletableFuture [StepResult] = getEdges (q) .toJava.toCompletableFuture `` ` > Create Interface for S2Edge/S2Vertex/S2Graph. > - > > Key: S2GRAPH-170 > URL: https://issues.apache.org/jira/browse/S2GRAPH-170 > Project: S2Graph > Issue Type: Improvement >Affects Versions: 0.2.1 >Reporter: DOYUNG YOON >Assignee: DOYUNG YOON > Fix For: 0.2.1 > > Original Estimate: 504h > Remaining Estimate: 504h > > h2. Problem Statement > S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class. > Even though lots of code touch theses two class, there is no interface > defined currently. This means lots of code is interact with these class in > different way all by their own way, and this make extremely hard to make any > change on these two classes. > For example, I was working on S2GRAPH-80 to provide java client, and there > are too many places to be changed since all theses places use concrete > implementation class S2Edge/S2Vertex/S2Graph, not the interfaces. Not just > for S2GRAPH-80, but any other issues that need to change theses classes would > benefit by communicating through interface. > h2. Suggestion > Define interface and change code base to communicate through theses > interfaces. > # Create interface S2Edge/S2Vertex/S2Graph that implement Tinkerpop’s > Edge/Vertex/Graph interface > # Extract tinkerpop interface related implementations > # Change caller of S2Edge/S2Vertex/S2Graph to call interface rather than > concrete implementation. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[
https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16248324#comment-16248324
]
DOYUNG YOON commented on S2GRAPH-170:
-
Worked on define S2Graph interface S2GraphLike, and first draft looks like
below.
{noformat}
trait S2GraphLike extends Graph {
implicit val ec: ExecutionContext
var apacheConfiguration: Configuration
protected val localLongId = new AtomicLong()
protected val s2Features = new S2Features
val config: Config
val management: Management
val indexProvider: IndexProvider
val elementBuilder: GraphElementBuilder
val traversalHelper: TraversalHelper
override def features() = s2Features
def fallback = Future.successful(StepResult.Empty)
def defaultStorage: Storage
def getStorage(service: Service): Storage
def getStorage(label: Label): Storage
def flushStorage(): Unit
def shutdown(modelDataDelete: Boolean = false): Unit
def getVertices(vertices: Seq[S2VertexLike]): Future[Seq[S2VertexLike]]
def checkEdges(edges: Seq[S2EdgeLike]): Future[StepResult]
def mutateVertices(vertices: Seq[S2VertexLike], withWait: Boolean = false):
Future[Seq[MutateResponse]]
def mutateEdges(edges: Seq[S2EdgeLike], withWait: Boolean = false):
Future[Seq[MutateResponse]]
def mutateElements(elements: Seq[GraphElement],
withWait: Boolean = false): Future[Seq[MutateResponse]]
def getEdges(q: Query): Future[StepResult]
def getEdgesMultiQuery(mq: MultiQuery): Future[StepResult]
def deleteAllAdjacentEdges(srcVertices: Seq[S2VertexLike],
labels: Seq[Label],
dir: Int,
ts: Long): Future[Boolean]
def incrementCounts(edges: Seq[S2EdgeLike], withWait: Boolean):
Future[Seq[MutateResponse]]
def updateDegree(edge: S2EdgeLike, degreeVal: Long = 0):
Future[MutateResponse]
def getVertex(vertexId: VertexId): Option[S2VertexLike]
def fetchEdges(vertex: S2VertexLike, labelNameWithDirs: Seq[(String,
String)]): util.Iterator[Edge]
def edgesAsync(vertex: S2VertexLike, direction: Direction, labelNames:
String*): Future[util.Iterator[Edge]]
/** Convert to Graph Element **/
def toVertex(serviceName: String,
columnName: String,
id: Any,
props: Map[String, Any] = Map.empty,
ts: Long = System.currentTimeMillis(),
operation: String = "insert"): S2VertexLike
def toEdge(srcId: Any,
tgtId: Any,
labelName: String,
direction: String,
props: Map[String, Any] = Map.empty,
ts: Long = System.currentTimeMillis(),
operation: String = "insert"): S2EdgeLike
def toGraphElement(s: String, labelMapping: Map[String, String] = Map.empty):
Option[GraphElement]
/** TinkerPop Interfaces **/
def vertices(ids: AnyRef*): util.Iterator[structure.Vertex]
def edges(edgeIds: AnyRef*): util.Iterator[structure.Edge]
def edgesAsync(edgeIds: AnyRef*): Future[util.Iterator[structure.Edge]]
def tx(): Transaction
def variables(): Variables
def configuration(): Configuration
def addVertex(label: String): Vertex
def makeVertex(idValue: AnyRef, kvsMap: Map[String, AnyRef]): S2VertexLike
def addVertex(kvs: AnyRef*): structure.Vertex
def addVertex(id: VertexId,
ts: Long = System.currentTimeMillis(),
props: S2Vertex.Props = S2Vertex.EmptyProps,
op: Byte = 0,
belongLabelIds: Seq[Int] = Seq.empty): S2VertexLike
/* tp3 only */
def addEdge(srcVertex: S2VertexLike, labelName: String, tgtVertex: Vertex,
kvs: AnyRef*): Edge
def close(): Unit
def compute[C <: GraphComputer](aClass: Class[C]): C
def compute(): GraphComputer
def io[I <: Io[_ <: GraphReader.ReaderBuilder[_ <: GraphReader], _ <:
GraphWriter.WriterBuilder[_ <: GraphWriter], _ <: Mapper.Builder[_]]](builder:
Io.Builder[I]): I
override def toString(): String
}
{noformat}
Now pretty much all of codes that used to directly use S2Edge/S2Vertex/S2Graph
in different way has been refactored so they only use interface defined on
S2EdgeLike/S2VertexLike/S2GraphLike.
All changes are available on
[PR|https://github.com/apache/incubator-s2graph/pull/127] .
> Create Interface for S2Edge/S2Vertex/S2Graph.
> -
>
> Key: S2GRAPH-170
> URL: https://issues.apache.org/jira/browse/S2GRAPH-170
> Project: S2Graph
> Issue Type: Improvement
>Affects Versions: 0.2.1
>Reporter: DOYUNG YOON
>Assignee: DOYUNG YOON
> Fix For: 0.2.1
>
> Original Estimate: 504h
> Remaining Estimate: 504h
>
> h2. Problem Statement
> S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class.
> Even though lots of code touch theses two
[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[
https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16241588#comment-16241588
]
DOYUNG YOON commented on S2GRAPH-170:
-
So far, I have removed SelfType on `S2EdgeLike/S2VertexLike` class(not yet
started with S2GraphLike). works can be found on
[here|https://github.com/SteamShon/incubator-s2graph/commits/S2GRAPH-170].
In summary following is how our interface looks like so far.
h4. S2VertexLike
{noformat}
trait S2VertexLike extends Vertex with GraphElement {
val graph: S2Graph
val id: VertexId
val ts: Long
val props: Props
val op: Byte
val belongLabelIds: Seq[Int]
val innerId = id.innerId
val innerIdVal = innerId.value
val builder = new S2VertexBuilder(this)
def label(): String = serviceColumn.columnName
def schemaVer = serviceColumn.schemaVersion
def serviceColumn = ServiceColumn.findById(id.colId)
def columnName = serviceColumn.columnName
lazy val service = Service.findById(serviceColumn.serviceId)
lazy val (hbaseZkAddr, hbaseTableName) = (service.cluster, service.hTableName)
def defaultProps: util.HashMap[String, S2VertexProperty[_]]
def toLogString(): String
def vertices(direction: Direction, edgeLabels: String*): util.Iterator[Vertex]
def edges(direction: Direction, labelNames: String*): util.Iterator[Edge]
def propertyInner[V](cardinality: Cardinality, key: String, value: V,
objects: AnyRef*): VertexProperty[V]
def property[V](cardinality: Cardinality, key: String, value: V, objects:
AnyRef*): VertexProperty[V]
def addEdge(labelName: String, vertex: Vertex, kvs: AnyRef*): Edge
def property[V](key: String): VertexProperty[V]
def properties[V](keys: String*): util.Iterator[VertexProperty[V]]
def remove(): Unit
}
{noformat}
h4. S2EdgeLike
{noformat}
trait S2EdgeLike extends Edge with GraphElement {
/* immutable variable */
val innerGraph: S2Graph
val srcVertex: S2VertexLike
var tgtVertex: S2VertexLike
val innerLabel: Label
val dir: Int
/* mutable variable */
var op: Byte
var version: Long
var tsInnerValOpt: Option[InnerValLike]
val propsWithTs: Props = S2Edge.EmptyProps
/* only internal usage variables */
val parentEdges: Seq[EdgeWithScore] = Nil
val originalEdgeOpt: Option[S2EdgeLike] = None
val pendingEdgeOpt: Option[S2EdgeLike] = None
val statusCode: Byte = 0
val lockTs: Option[Long] = None
lazy val ts: Long
private lazy val operation: String
private lazy val direction: String
private lazy val tsInnerVal: Any
/* getter/setter */
def graph(): Graph
lazy val edgeId: EdgeId
def id(): AnyRef
def label(): String
def getLabelId(): Int
def getDirection(): String
def getOperation(): String
def getTsInnerValValue(): Any
def isDirected(): Boolean
def getTs(): Long = ts
def getOriginalEdgeOpt(): Option[S2EdgeLike]
def getParentEdges(): Seq[EdgeWithScore]
def getPendingEdgeOpt(): Option[S2EdgeLike]
def getPropsWithTs(): Props
def getLockTs(): Option[Long]
def getStatusCode(): Byte
def getDir(): Int
def setTgtVertex(v: S2VertexLike): Unit
def getOp(): Byte
def setOp(newOp: Byte): Unit
def getVersion(): Long
def setVersion(newVersion: Long): Unit
def getTsInnerValOpt(): Option[InnerValLike]
def setTsInnerValOpt(newTsInnerValOpt: Option[InnerValLike]): Unit
/* conversion helper used internal */
def toSnapshotEdge: SnapshotEdge
def toIndexEdge(labelIndexSeq: Byte): IndexEdge
/* property related helper */
def updatePropsWithTs(others: Props = S2Edge.EmptyProps): Props
def propertyValue(key: String): Option[InnerValLikeWithTs]
def propertyValueInner(labelMeta: LabelMeta): InnerValLikeWithTs
def propertyValues(keys: Seq[String] = Nil): Map[LabelMeta,
InnerValLikeWithTs]
def propertyValuesInner(labelMetas: Seq[LabelMeta] = Nil): Map[LabelMeta,
InnerValLikeWithTs]
/* copy helper */
def checkProperty(key: String): Boolean
def copyEdgeWithState(state: State): S2EdgeLike
def copyOp(newOp: Byte): S2EdgeLike
def copyVersion(newVersion: Long): S2EdgeLike
def copyParentEdges(parents: Seq[EdgeWithScore]): S2EdgeLike
def copyOriginalEdgeOpt(newOriginalEdgeOpt: Option[S2EdgeLike]): S2EdgeLike
def copyStatusCode(newStatusCode: Byte): S2EdgeLike
def copyLockTs(newLockTs: Option[Long]): S2EdgeLike
def copyTs(newTs: Long): S2EdgeLike
def updateTgtVertex(id: InnerValLike): S2EdgeLike
/* tinkerpop Edge interface */
def vertices(direction: Direction): util.Iterator[structure.Vertex]
def properties[V](keys: String*): util.Iterator[Property[V]]
def property[V](key: String): Property[V]
def property[V](key: String, value: V): Property[V]
def propertyInner[V](key: String, value: V, ts: Long): Property[V]
def remove(): Unit
/* GraphElement interface */
def toLogString: String
}
{noformat}
There are many methods
[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[ https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16240094#comment-16240094 ] Daewon Jeong commented on S2GRAPH-170: -- I reviewed the above draft. I agree to proceed with refactoring to clarify the interface. However, I have some comments on this refactoring. # S2 [Vertex, Edge] Like 'SelfType' is used in the 'S2Vertex` and' S2VertexLike 'as there is a bidirectional dependency. S2VertexLike should be unaware of the existence of S2Vertex because the dependency should flow unidirectionally. To achieve the above goal, `SelfType` should be removed. p.s: This refactoring seems to be in gradual improvement for compatibility with existing code (not to break caller). > Create Interface for S2Edge/S2Vertex/S2Graph. > - > > Key: S2GRAPH-170 > URL: https://issues.apache.org/jira/browse/S2GRAPH-170 > Project: S2Graph > Issue Type: Improvement >Affects Versions: 0.2.1 >Reporter: DOYUNG YOON > Fix For: 0.2.1 > > Original Estimate: 504h > Remaining Estimate: 504h > > h2. Problem Statement > S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class. > Even though lots of code touch theses two class, there is no interface > defined currently. This means lots of code is interact with these class in > different way all by their own way, and this make extremely hard to make any > change on these two classes. > For example, I was working on S2GRAPH-80 to provide java client, and there > are too many places to be changed since all theses places use concrete > implementation class S2Edge/S2Vertex/S2Graph, not the interfaces. Not just > for S2GRAPH-80, but any other issues that need to change theses classes would > benefit by communicating through interface. > h2. Suggestion > Define interface and change code base to communicate through theses > interfaces. > # Create interface S2Edge/S2Vertex/S2Graph that implement Tinkerpop’s > Edge/Vertex/Graph interface > # Extract tinkerpop interface related implementations > # Change caller of S2Edge/S2Vertex/S2Graph to call interface rather than > concrete implementation. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[ https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16240119#comment-16240119 ] Daewon Jeong commented on S2GRAPH-170: -- [~steamshon] S2 [Edge / Vertex / Graph] In order to derive the exposed interface from Like, it seems to be necessary to remove the `SelfType` and then move the method to the actual implementation. Depending on the function, once the methods are moved to the appropriate Module, we think that only the required interface will be left. This is a painful task, but it is expected to result in improved code quality. > Create Interface for S2Edge/S2Vertex/S2Graph. > - > > Key: S2GRAPH-170 > URL: https://issues.apache.org/jira/browse/S2GRAPH-170 > Project: S2Graph > Issue Type: Improvement >Affects Versions: 0.2.1 >Reporter: DOYUNG YOON > Fix For: 0.2.1 > > Original Estimate: 504h > Remaining Estimate: 504h > > h2. Problem Statement > S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class. > Even though lots of code touch theses two class, there is no interface > defined currently. This means lots of code is interact with these class in > different way all by their own way, and this make extremely hard to make any > change on these two classes. > For example, I was working on S2GRAPH-80 to provide java client, and there > are too many places to be changed since all theses places use concrete > implementation class S2Edge/S2Vertex/S2Graph, not the interfaces. Not just > for S2GRAPH-80, but any other issues that need to change theses classes would > benefit by communicating through interface. > h2. Suggestion > Define interface and change code base to communicate through theses > interfaces. > # Create interface S2Edge/S2Vertex/S2Graph that implement Tinkerpop’s > Edge/Vertex/Graph interface > # Extract tinkerpop interface related implementations > # Change caller of S2Edge/S2Vertex/S2Graph to call interface rather than > concrete implementation. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[ https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16240115#comment-16240115 ] DOYUNG YOON commented on S2GRAPH-170: - [~daewon] I agree with getting rid of bidirectional dependency. Before removing it, I want to discuss about the direction we should take. As personal opinion, I think S2[Edge/Vertex/Graph]Like should be pure interface, with very little implementation. I would like to list up methods that really need to be exposed on S2[Edge/Vertex/Graph]Like(I mean interface) first, then proceed removing `SelfType` as you pointed out. > Create Interface for S2Edge/S2Vertex/S2Graph. > - > > Key: S2GRAPH-170 > URL: https://issues.apache.org/jira/browse/S2GRAPH-170 > Project: S2Graph > Issue Type: Improvement >Affects Versions: 0.2.1 >Reporter: DOYUNG YOON > Fix For: 0.2.1 > > Original Estimate: 504h > Remaining Estimate: 504h > > h2. Problem Statement > S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class. > Even though lots of code touch theses two class, there is no interface > defined currently. This means lots of code is interact with these class in > different way all by their own way, and this make extremely hard to make any > change on these two classes. > For example, I was working on S2GRAPH-80 to provide java client, and there > are too many places to be changed since all theses places use concrete > implementation class S2Edge/S2Vertex/S2Graph, not the interfaces. Not just > for S2GRAPH-80, but any other issues that need to change theses classes would > benefit by communicating through interface. > h2. Suggestion > Define interface and change code base to communicate through theses > interfaces. > # Create interface S2Edge/S2Vertex/S2Graph that implement Tinkerpop’s > Edge/Vertex/Graph interface > # Extract tinkerpop interface related implementations > # Change caller of S2Edge/S2Vertex/S2Graph to call interface rather than > concrete implementation. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (S2GRAPH-170) Create Interface for S2Edge/S2Vertex/S2Graph.
[ https://issues.apache.org/jira/browse/S2GRAPH-170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16238525#comment-16238525 ] DOYUNG YOON commented on S2GRAPH-170: - Initial draft is available on https://github.com/SteamShon/incubator-s2graph/tree/S2GRAPH-170. Please feel free to comment. please suggest better name than S2EdgeLike/S2VertexLike/S2GraphLike. > Create Interface for S2Edge/S2Vertex/S2Graph. > - > > Key: S2GRAPH-170 > URL: https://issues.apache.org/jira/browse/S2GRAPH-170 > Project: S2Graph > Issue Type: Improvement >Affects Versions: 0.2.1 >Reporter: DOYUNG YOON >Priority: Major > Fix For: 0.2.1 > > Original Estimate: 504h > Remaining Estimate: 504h > > h2. Problem Statement > S2Graph’s entire code base is dependent on S2Edge/S2Vertex/S2Graph class. > Even though lots of code touch theses two class, there is no interface > defined currently. This means lots of code is interact with these class in > different way all by their own way, and this make extremely hard to make any > change on these two classes. > For example, I was working on S2GRAPH-80 to provide java client, and there > are too many places to be changed since all theses places use concrete > implementation class S2Edge/S2Vertex/S2Graph, not the interfaces. Not just > for S2GRAPH-80, but any other issues that need to change theses classes would > benefit by communicating through interface. > h2. Suggestion > Define interface and change code base to communicate through theses > interfaces. > # Create interface S2Edge/S2Vertex/S2Graph that implement Tinkerpop’s > Edge/Vertex/Graph interface > # Extract tinkerpop interface related implementations > # Change caller of S2Edge/S2Vertex/S2Graph to call interface rather than > concrete implementation. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
