Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework dashboard to work through JSON RPC protocol #5952

Merged
merged 1 commit into from
Aug 16, 2017
Merged

Conversation

ashumilova
Copy link
Contributor

@ashumilova ashumilova commented Aug 9, 2017

Signed-off-by: Anna Shumilova ashumilo@redhat.com

What does this PR do?

Reworks dashboard to us JSON RPC protocol. The left stuff - workspace diagnostic - depends on #5935, also founded following missing part: #5932

What issues does this PR fix or reference?

#4763
#5358

Changelog

Adapted dashboard to use JSON RPC protocol for master and agent interactions.

Release Notes

N/A

Docs PR

N/A

@ashumilova ashumilova added this to the 5.17.0 milestone Aug 9, 2017
@ashumilova ashumilova self-assigned this Aug 9, 2017

/**
* Performs connection to the pointed entrypoint.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add a param: @param entrypoint {string}

startWorkspace: {method: 'POST', url : authData.url + '/api/workspace/:workspaceId/runtime?environment=:envName&token=' + authData.token}
}
);
}

createWorkspaceFromConfig(accountId, workspaceConfig) {
return this.remoteWorkspaceAPI.create({accountId : accountId}, workspaceConfig).$promise;
createWorkspaceFromConfig(workspaceConfig: any) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add the return type

*/
getMachineToken(workspaceId) {
*/
getMachineToken(workspaceId: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return type

@@ -65,29 +80,26 @@ export class CheRemoteWorkspace {
* @param envName the name of the environment
* @returns {*} promise
*/
startWorkspace(remoteWsURL, workspaceId, envName) {

startWorkspace(remoteWsURL: string, workspaceId: string, envName: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return type

@@ -38,7 +41,7 @@ export class CheRemote {
* @param password the password on the remote server
* @returns {*|promise|N|n}
*/
newAuth(url, login, password) {
newAuth(url: string, login: string, password: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return type

@@ -49,8 +52,8 @@ export class CheRemote {
* @param token
* @returns {*}
*/
newWorkspace(remoteConfig) {
return new CheRemoteWorkspace(this.$resource, this.$q, this.cheWebsocket, remoteConfig);
newWorkspace(remoteConfig: any) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return type

@codenvy-ci
Copy link

@codenvy-ci
Copy link

@@ -635,6 +637,10 @@ export class CheWorkspace {
return this.workspaceSettings ? this.workspaceSettings['che.workspace.auto_snapshot'] === 'true' : true;
}

public getMasterApi(): CheJsonRpcMasterApi {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO I was thinking it was returning something else when I saw getMasterApi() in the code
It's because in che workspace we're already connected to the "master" node so it is looking odd to me to get a "master api" while it's mostly to handle RPC calls.

@@ -646,4 +652,22 @@ export class CheWorkspace {
this.workspacesById.set(workspace.id, workspace);
this.startUpdateWorkspaceStatus(workspace.id);
}

private connectWsMasterApi($location: ng.ILocationService, proxySettings : string, devmode: boolean): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the pattern is : new CheJsonRPCMasterAPI(websocketClient)
and then : rpcMasterAPI.connect(endpoint)
IMHO we're mixing two patterns because object is not useable until we're connected.
could we have it in a more "factory pattern" ?
like we get a new dedicated object each time we connect on an endpoint (and it may be pooled as well)

factory = new RPCJsonFactory(configuration)
factory.get(endpoint) --> return instance of CheJsonRpcMasterApi

@ashumilova ashumilova force-pushed the che-4763 branch 2 times, most recently from ccc5906 to fa772d2 Compare August 16, 2017 11:02
Signed-off-by: Anna Shumilova <ashumilo@redhat.com>
@codenvy-ci
Copy link

Build # 3408 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/3408/ to view the results.

@slemeur slemeur added the kind/enhancement A feature request - must adhere to the feature request template. label Oct 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants