mta set credentials for application #446
Conversation
RichardW98
force-pushed
the
mta-credentials
branch
from
0a69d7c
to
aca931c
Compare
| @@ -105,7 +107,9 @@ else if (result instanceof Result.Success<TaskGroup> success) { | |||
| "[Migration analysis report](%s) completed.".formatted(reportURL)); | |||
| return new DefaultWorkReport(WorkStatus.COMPLETED, workContext); | |||
| } | |||
| else if ("Failed".equals(success.value().state())) { | |||
| else if ("Failed".equals(success.value().state()) | |||
| || Arrays.stream(Objects.requireNonNull(success.value().tasks())) | |||
There was a problem hiding this comment.
isn't a failed task sets the top-level taskgroup as failed? what is the state of the taskgroup in that case? (weird API if so)
There was a problem hiding this comment.
I tried to create application with the same name of existing one. the top level is succeed but task is failed
| @@ -51,6 +54,8 @@ interface MTAApplicationClient { | |||
|
|
|||
| Result<App> create(App app); | |||
|
|
|||
| Identity getIdentity(String name); | |||
There was a problem hiding this comment.
can you make that a Result ? the intention is among other not to throw execptions from the client level. They should be stored in the Result of type Failure.
| @@ -86,10 +86,12 @@ WorkFlowOption defaultOption() { | |||
| @Bean(name = "AnalyzeApplicationAssessment") | |||
| @Assessment(parameters = { | |||
| @Parameter(key = "repositoryURL", description = "The repository with the code to analyze", | |||
| type = WorkParameterType.URL, optional = false), | |||
| type = WorkParameterType.TEXT, optional = false), | |||
There was a problem hiding this comment.
why not URL? because of the protocol that can be ssh? if that's the case I suggest to replace URL with URI which does support that, or at least behind the hood to use URI as the resolver
There was a problem hiding this comment.
Java URI support opaque schemes . URI.create("foo://bar.baz").getScheme() // foo"
There was a problem hiding this comment.
ok let's replace URL with URI in next pr. it involves UI work too
There was a problem hiding this comment.
UI's pattern for url is "^(https?)://"
| @@ -15,7 +15,7 @@ public static String getPath(String server, String workspaceID, String projectID | |||
| throws URISyntaxException { | |||
| String path = "/workspaces/%s/projects/%s/outputs/%s".formatted(workspaceID, projectID, outputID); | |||
| URI baseUri = new URI(server); | |||
| return new URI(baseUri.getScheme(), baseUri.getAuthority(), path, null, null).getPath(); | |||
| return new URI(baseUri.getScheme(), baseUri.getAuthority(), path, null, null).toString(); | |||
There was a problem hiding this comment.
No need to update the test?
There was a problem hiding this comment.
i will update it
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rgolangh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Change type
Impacted services
Checklist