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

Exec agent Che IDE client #3032

Closed
wants to merge 33 commits into from
Closed

Exec agent Che IDE client #3032

wants to merge 33 commits into from

Conversation

dkuleshov
Copy link

@dkuleshov dkuleshov commented Nov 9, 2016

What does this PR do?

  • ExecAgentCommandManager - new exec agent Che IDE client
  • Improvements/fixes/changes to JsonRPC protocol
  • Improvements/fixes/changes to websocket protocol

What issues does this PR fix or reference?

This is an adaptation of a new exec agent implementation located in exec-agent branch, the work is done under #1947 issue.

Changes to existing protocols

Websocket protocol

  1. Websocket implementation on IDE side now support custom endpoints. This is done by calling improved initializer:
    org.eclipse.che.ide.websocket.ng.impl.WebSocketInitializer#initialize(String endpointId, String url)
    where
  • endpointId is a high level websocket connection identifier (e.g. "exec-agent", "ws-agent", etc.)
  • url is a URL of websocket endpoint the connection is established to.
    After a connection is initialized and established an endpoint identifier can be used to choose where to send a message via a transmitter, an instance of org.eclipse.che.ide.websocket.ng.WebSocketMessageTransmitter#transmit(String endpointId, String message)
  1. Multi-protocol support is considered redundant and removed, now websocket supports only json rpc protocol transfers, though core routines for addition of support of new protocols are still present.
  2. Websocket messages are now plain text messages thus parsing and analyzing is moved to a higher level consumers (e.g. json rpc service dispatchers).

JsonRPC protocol

  1. Added automation on different levels, requests and responses are now parsed automatically and the business objects are now passed to actual receivers and transmitters.
  2. As client-side websocket implementation supports multiple endpoints json rpc receivers for client side now requires a new way of binding in a dependency injection container. This is now done like this:
GinMapBinder.newMapBinder(binder(), String.class, RequestHandler.class)
                    .addBinding("exec-agent" + '@' + "connected")
                    .to(ConnectedEventHandler.class);

where

  • exec-agent is an endpoint identifier
  • connected is a method name in terms of JsonRpc 2.0 protocol.
    At the same time for server-side registration procedure nothing is changed.
  1. Changed API of request and notification receivers. At the moment it is done by extending of org.eclipse.che.ide.jsonrpc.RequestHandler class for client-side and
    org.eclipse.che.api.core.jsonrpc.RequestHandler for server-side. The request handlers are now responsible for handling all kind of incoming request and notification. Handling is done by overriding of a corresponding method. Classes have two generic type parameters <P, R> that corresponds for request/notification parameters and response result class types. Note that for handling requests the implementor isn't responsible now for response transmission because it is done automatically by taking the return value of a corresponding method.
  2. Changed API of request and notification transmitters. This is now done by calling instances of
    org.eclipse.che.api.core.jsonrpc.RequestTransmitter and org.eclipse.che.ide.jsonrpc.RequestTransmitter for server side and client side correspondingly. Now request transmission methods return javascript promise wrappers for client-side and completable futures for server-side. That allows to dynamically handle response while previously was done by static binding of a corresponding response receiver in a dependency injection container.

New exec agent client capabilities and use-cases

This is the core feature of this pull request, main task is to add a handy client to address process related calls to an exec agent through json rpc protocol.
Original json rpc exec agent API:

The client is added as an implementation of org.eclipse.che.ide.api.machine.ExecAgentCommandManager and provide next methods

    /**
     * Call exec agent to start a process with specified command parameters
     *
     * @param command command
     * @return exec agent promise with appropriate dto
     */
    ExecAgentPromise<ProcessStartResponseDto> startProcess(Command command);

    /**
     * Call exec agent to kill a process with specified identifier
     *
     * @param pid process identifier
     * @return promise with appropriate dto
     */
    Promise<ProcessKillResponseDto> killProcess(int pid);

    /**
     * Call for a subscription to events related to a specified process after defined timestamp
     * represented by a corresponding string (RFC3339Nano e.g. "2016-07-26T09:36:44.920890113+03:00").
     *
     * @param pid process identifier
     * @param eventTypes event types (e.g. stderr, stdout)
     * @param after after timestamp
     * @return exec agent promise with appropriate dto
     */
    ExecAgentPromise<ProcessSubscribeResponseDto> subscribe(int pid, List<String> eventTypes, String after);

    /**
     * Call for a cancellation of a subscription to events related to a specific process after defined
     * timestamp represented by a corresponding string (RFC3339Nano e.g. "2016-07-26T09:36:44.920890113+03:00").
     *
     * @param pid process identifier
     * @param eventTypes event types (e.g. stderr, stdout)
     * @param after after timestamp
     * @return promise with appropriate dto
     */
    Promise<ProcessUnSubscribeResponseDto> unsubscribe(int pid, List<String> eventTypes, String after);

    /**
     * Call for an update of a subscription to events related to a specific process.
     *
     * @param pid process identifier
     * @param eventTypes event types (e.g. stderr, stdout)
     * @return promise with appropriate dto
     */
    Promise<UpdateSubscriptionResponseDto> updateSubscription(int pid, List<String> eventTypes);

    /**
     * Call for a report on proess logs of a specific process.
     *
     * @param pid process identifier
     * @param from string represented timestamp the beginning of a time
     *             segment (RFC3339Nano e.g. "2016-07-26T09:36:44.920890113+03:00")
     * @param till string represented timestamp the ending of a time
     *             segment (RFC3339Nano e.g. "2016-07-26T09:36:44.920890113+03:00")
     * @param limit the limit of logs in result, the default value is 50
     * @param skip the logs to skip, default value is 0
     * @return promise with appropriate dto
     */
    Promise<List<GetProcessLogsResponseDto>> getProcessLogs(int pid, String from, String till, int limit, int skip);

    /**
     * Call for a process info
     *
     * @param pid process identifier
     * @return promise with appropriate dto
     */
    Promise<GetProcessResponseDto> getProcess(int pid);

    /**
     * Call for a processes info
     *
     * @param all defines if include already stopped processes, true for all process, 
     * false for running
     * @return promise with appropriate dto
     */
    Promise<List<GetProcessesResponseDto>> getProcesses(boolean all);

As you can see this API requires support of optional parameters and it will be added soon but now it is quite enough for all actual workflows and probably the most simple and basic one should be the following:

  1. We ask exec agent to start a process -> request
  2. Exec agent starts a process and provide process start related information <- response
  3. Exec agent automatically subscribe us for all process related events (start, stdout, stderr, finish, etc.)
  4. Each time an event happens we receive a message from an exec agent <- notification
  5. In order to react on event we can register event operations by using either org.eclipse.che.ide.api.machine.ExecAgentEventManager injecting it or org.eclipse.che.ide.api.machine.execagent.ExecAgentPromise that is retuned by corresponding methods (startProcess, subscribe).
  6. We ask exec agent to stop a process -> request
  7. Exec agent stops a process and provide process stop related information <- response

A good example of how to run a command using the new client is a piece of org.eclipse.che.ide.extension.machine.client.actions.RunExecAgentCommandsAction source code:

Command command = new CommandImpl("Test command", value, "Custom");
jsonRpcExecAgentCommandManager.startProcess(command)
                  .thenIfProcessStartedEvent(new ProcessStartedEventOperation(console))
                  .thenIfProcessDiedEvent(new ProcessDiedEventOperation(console))
                  .thenIfProcessStdOutEvent(new ProcessStdOutEventOperation(console))
                  .thenIfProcessStdErrEvent(new ProcessStdErrEventOperation(console));

where

  • "Test command" is the name of the command
  • value is the command line
  • "Custom" is the type of the command
  • Process*EventOperation operation that will be performed on project (started, died, stdout, stderr ) event

@dkuleshov dkuleshov added the status/in-progress This issue has been taken by an engineer and is under active development. label Nov 9, 2016
@dkuleshov dkuleshov self-assigned this Nov 9, 2016
Signed-off-by: Dmitry Kuleshov <dkuleshov@codenvy.com>
@codenvy-ci
Copy link

@codenvy-ci
Copy link

* Dispatches json rpc notification received from endpoint identified by a
* high level identifier and represented as a json object.
*
* @param endpointId high level endpoint identifier
Copy link
Member

Choose a reason for hiding this comment

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

javadoc isn't formatted with che code style.
Please, review all classes in org.eclipse.che.api.core.jsonrpc.impl package because another ones have the same problem.

import javax.inject.Singleton;

/**
* Dispatches messages received from web socket endpoint throughout json rcp
Copy link
Member

Choose a reason for hiding this comment

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

json rcp -> json rpc

import javax.inject.Singleton;

/**
* Dispatches messages received from web socket endpoint throughout json rcp
Copy link
Member

Choose a reason for hiding this comment

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

json rcp -> json rpc


/**
* Register an operation which will be performed when a process generates
* process standard error event.
Copy link
Member

Choose a reason for hiding this comment

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

Doc says standard error event but method name is thenIfProcessStdOutEvent.
I think that doc should say standard output event.
No?

Copy link
Author

Choose a reason for hiding this comment

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

You're absolutely right


/**
* Register an operation which will be performed when a process generates
* process standard output event.
Copy link
Member

Choose a reason for hiding this comment

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

Doc says standard output event but method name is thenIfProcessStdErrEvent.
I think that doc should say standard error event.
No?

Copy link
Author

Choose a reason for hiding this comment

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

Correct

* Copyright (c) 2012-2016 Codenvy, S.A.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
Copy link
Member

Choose a reason for hiding this comment

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

I think that one of these license headers is unnecessary )
And please review all new DTOs in this PR. Because really a lot of them have duplicated license headers.

* @author Dmitry Kuleshov
*/
@DTO
public interface ProcessStartResponseDto extends DtoWithPidDto{
Copy link
Member

Choose a reason for hiding this comment

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

Line isn't formatted. There is no space before {

*/
void transmit(JsonRpcRequest request);
@DTO
public interface ProcessKillResponseDto extends DtoWithPidDto{
Copy link
Member

Choose a reason for hiding this comment

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

Line isn't formatted. There is no space before {

* Dispatches json rpc notification received from endpoint identified by a
* high level identifier and represented as a json object.
*
* @param endpointId high level endpoint identifier
Copy link
Member

Choose a reason for hiding this comment

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

javadoc isn't formatted with che code style.
Please, review all classes in org.eclipse.che.ide.jsonrpc.impl package because several of them have the same problem.

Dmitry Kuleshov and others added 5 commits November 16, 2016 13:47
…for endusers

Signed-off-by: Dmitry Kuleshov <dkuleshov@codenvy.com>
Signed-off-by: Dmitry Kuleshov <dkuleshov@codenvy.com>
@codenvy-ci
Copy link

Build # 1050 - FAILED

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

Signed-off-by: Dmitry Kuleshov <dkuleshov@codenvy.com>
@codenvy-ci
Copy link

Build # 1071 - FAILED

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

@vparfonov vparfonov added sprint/next team/ide status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. and removed status/in-progress This issue has been taken by an engineer and is under active development. labels Nov 24, 2016
@voievodin voievodin force-pushed the exec-agent branch 2 times, most recently from 19a4927 to a34f616 Compare December 1, 2016 16:55
@codenvy-ci
Copy link

Build # 1193 - FAILED

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

@codenvy-ci
Copy link

Build # 1232 - FAILED

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

@voievodin voievodin force-pushed the exec-agent branch 2 times, most recently from a79f9ba to 6fba10b Compare December 8, 2016 13:36
@codenvy-ci
Copy link

Build # 1262 - FAILED

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

@TylerJewell
Copy link

@dkuleshov - now that we have tagged 5.0.0 - can you resolve these conflicts and be ready to merge this? I think it's good for us to clean up our backlog of PRs, before moving onto bigger projects.

@dkuleshov
Copy link
Author

This is merged to master under another pull request

@dkuleshov dkuleshov closed this Jan 13, 2017
@dkuleshov dkuleshov deleted the exec-agent-adapt branch January 13, 2017 15:34
@JamesDrummond JamesDrummond mentioned this pull request Feb 17, 2017
9 tasks
@benoitf benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants