issue42 Remove references to coordinator endpoints from the LRAClient API #68
Conversation
|
@mmusgrov Just to clarify my intent here - we don't want to discourage any implementation to base LRA on decentralized self-organized solution, however, this PR requires that all implementations based on the orchestration will be configured in the same way. Is it ok to fix the issue? |
| @@ -120,14 +119,9 @@ private static void initTck(LRAClient lraClient) { | |||
| Thread.sleep(1000); | |||
| } | |||
|
|
|||
| int servicePort = Integer.getInteger("service.http.port", TEST_SWARM_PORT); | |||
There was a problem hiding this comment.
"service.http.port" is the config for where the TCK test resources (ActivityController etc) are deployed. I know that in the Narayana case our coordinator and these resources are both deployed to the same server and port but they don't need to be. Keeping it configurable would allow another vendor to run the TCK resources separately from whatever mechanism they use to configure their LRAClient implementation.
But clearly we would need a better config names, maybe something like:
@Inject @ConfigProperty(name="lra.tck.http.port", defaultValue="8080")
private int servicePort;There was a problem hiding this comment.
@mmusgrov ok, I will revert this line.
| micrserviceBaseUrl = new URL(String.format("http://localhost:%d", servicePort)); | ||
| rcBaseUrl = new URL(String.format("http://%s:%d/%s", rcHost, rcPort, rcPath)); | ||
| String dafaultUrl = "http://localhost:8080"; | ||
| micrserviceBaseUrl = new URL(System.getProperty(LRA_HTTP_URL, dafaultUrl)); |
There was a problem hiding this comment.
See my other comment about the use of the service.http.port property
| * Key for looking up the config property that specifies which host a | ||
| * coordinator is running on | ||
| * The config property that specifies a URL exposed for LRA orchestration and | ||
| * management. An implementation is not required to make use of this property. | ||
| */ |
There was a problem hiding this comment.
will do. However, for the javadoc is this sufficient? Please see my comment in the PR - #68 (comment)
There was a problem hiding this comment.
This bit of config is so that the TCK knows when the implementation has recovered. We briefly discussed this yesterday ("Are we going to specify recovery semantics in the spec? ")
We said that we would propose and SPI that implementations would implement in order to pass the TCK. In the LRA hangout a week last Monday I proposed something like:
public ManagementSPI {
LRAInfo getStatus(LRA lra); // get status of one LRA
Collection<LRAInfo> getLRAs(CompensatorStatus status); // list LRAs in the given state
//anything else
}So to get recovering LRAs the status would be set to Completing or Compensating.
This would mean we would not need to configure the endpoint in the TCK.
mmusgrov
added
the
Advertise
|
I've decided to close this PR in favor of the transition to MP config property configuration handling (#62) |
|
After a deeper investigation of the issue #62 MP config inclusion part, it seems that the fix for #62 will not affect the spec code only text part and the TCK. For this reason, I think this PR has still value in merging as it will decrease number of properties that needs to be configured in the TCK and described in the spec text. |
mmusgrov
added
stalled
xstefank
changed the title
… API Signed-off-by: xstefank <[email protected]>
|
Closing since issue #139 is now merged |
issue #42
Signed-off-by: xstefank [email protected]
The specification text will be adjusted as a part of the fix for issue #62 with the move to microprofile config.