aglinxinyuan commented on code in PR #4490:
URL: https://github.com/apache/texera/pull/4490#discussion_r3198360902


##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/state/State.scala:
##########
@@ -57,6 +58,9 @@ object State {
 
   def fromTuple(row: Tuple): State = fromJson(row.getField[String](Content))
 
+  def uriFromResultUri(resultUri: URI): URI =

Review Comment:
   Done in 4223cf56 — switched to a base-URI design. 
`OutputPortConfig.storageURI` (and the URI through `AssignPortRequest`) now 
carries the port *base* URI; result and state URIs are equal first-class 
derivatives off it via `VFSURIFactory.resultURI(base)` / `stateURI(base)`. No 
more `siblingStateURI`, no more substring replace, and no more "derive one URI 
from another" — both are built from the same `(workflowId, executionId, 
globalPortId)` base.



##########
amber/src/main/python/core/models/state.py:
##########
@@ -41,6 +41,10 @@ def from_json(cls, payload: str) -> "State":
     def from_tuple(cls, row: Tuple) -> "State":
         return cls.from_json(row[cls.CONTENT])
 
+    @staticmethod
+    def uri_from_result_uri(result_uri: str) -> str:

Review Comment:
   Same in Python — `VFSURIFactory.result_uri(base)` / `state_uri(base)` mirror 
the Scala API; `State.uri_from_result_uri` is gone.



##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/state/State.scala:
##########
@@ -57,6 +58,9 @@ object State {
 
   def fromTuple(row: Tuple): State = fromJson(row.getField[String](Content))
 
+  def uriFromResultUri(resultUri: URI): URI =
+    new URI(resultUri.toString.replace("/result", "/state"))

Review Comment:
   Resolved in 4223cf56 — replaced the `.replace` helper with a base-URI 
design: `VFSURIFactory.resultURI(base)` / `stateURI(base)` build both URIs from 
a shared port base URI, no string surgery.



##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/state/State.scala:
##########
@@ -57,6 +58,9 @@ object State {
 
   def fromTuple(row: Tuple): State = fromJson(row.getField[String](Content))
 
+  def uriFromResultUri(resultUri: URI): URI =
+    new URI(resultUri.toString.replace("/result", "/state"))
+

Review Comment:
   Resolved in 4223cf56 — replaced the `.replace` helper with a base-URI 
design: `VFSURIFactory.resultURI(base)` / `stateURI(base)` build both URIs from 
a shared port base URI, no string surgery.



##########
amber/src/main/python/core/models/state.py:
##########
@@ -41,6 +41,10 @@ def from_json(cls, payload: str) -> "State":
     def from_tuple(cls, row: Tuple) -> "State":
         return cls.from_json(row[cls.CONTENT])
 
+    @staticmethod
+    def uri_from_result_uri(result_uri: str) -> str:
+        return result_uri.replace("/result", "/state")

Review Comment:
   Resolved in 4223cf56 — same fix on the Python side. Helper deleted; 
result/state URIs are now built from a shared base via 
`VFSURIFactory.result_uri` / `state_uri`.



-- 
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]

Reply via email to