Community
Participate
Working Groups
Currently, Remote Tools View supports the hierarchy like: ITargetType --> ITargetElement. ITargetType is extensible, so we can add new target type like the following: |--Generic Host | |--Remote Host | |--MyTargetType(ITargetType) | |-->MyTargetElement(ITargetElement) We have two proposals to make Remote Tools View more extensible. 1) We want to use Remote Tools View to display remote connections and other things related to remote stuffs, like configuration for remote tool launch and output files from launch. The current two-level hierarchy prevents us from putting more into this view. So we would like have an extension point for extending ITargetElement. With the extension point, we can define a kind of TargetElement with children elements, and have more complicated hierarchies in Remote Tools View. It will look like: |--Generic Host | |--Remote Host | |--MyTargetType(ITargetType) | |-->MyTargetElement(ITargetElement) | |-->Configuration for Remote Tool Launch | |-->Output Files from Remote Tool Launch | |-->Log Files from Remote Tool Launch So the Toolbar Actions (Start/Stop/Resume/Pause) need to be extensible too, so that its behavior differs depending on what user selects in the view. This could be done through either Adapter pattern or define an extension point. It allows contributors to extend different Actions identified by the class type of selected element. 2) The individual tool need create its own ITargetType in Remote Tools View. So, now, Remote Tools Framework has Generic Host, and PTP also has Remote Host as top level in the view. Another proposal is: Can we have only one ITargetType "Generic Host"(contributed by Remote Tools itself) as a common point and user can create remote connection under it? Contributors can contribute their own nodes under connection. If so the connection can be shared by all contributors, and it's unnecessary for user to create a lot of duplicate connections for different tools. So the Remote Tools View could be: |--Generic Host | |-->Remote Host1 | |-->Remote Host2(ITargetElement) | |-->MyChildElement | |-->MyGrandSonElement1 | |-->MyGrandSonElement2 We're going to try to have some patch for the first proposal soon.
I needed to create a Remote Host in PTP in order to access information (in particular, attributes such as the host and user name) that were not exposed in the Generic Host. It would be nice to not have to do this, but it would mean that this information would also need to be exposed in the Generic Host.
Created attachment 98135 [details] The patch to demostrate the proposal The patch includes 4 plugins: com.zj.perftarget org.eclipse.ptp.remotetools.environment.core org.eclipse.ptp.remotetools.environment.launcher org.eclipse.ptp.remotetools.environment.ui The com.zj.perftarget demostrates how to contribute target element and tool bar action. The other three plugins contains the changes needed to implement the extension framework.
The "Generic Host" ITargetType was intended to provide connections for all tools and launchers. Therefore, a new tool should not create a new ITagetType for its own. Even more, a tool should work any ITargetElement created from any ITargetControl. That is how Remote Tools was designed to work. A new ITagetType should be created only when the connection strategy is different for some particular type of machine. For example, a new ITagetType would be a simulator which lifecycle needs to be controlled. But all tools and launchers should be able to work on the simulator, too. As Greg had to create a new ITargetType called "Remote Host" and that is nearly the same as "Generic Host", I believe that we should merge them into only one ITargetType that inter-operates and integrates better with other plugins, like PTP itself. I have another concern about extending the ITargetType with child elements. Although I think that the proposed extension is a potential for better presenting information to the user, I am afraid of: - Overpopulating the target view with information and features that are not related with the view's original purpose, that is managing target lifecycle. - Reimplementing features that are already available in Eclipse and that the user expects to find elsewhere. For example, Eclipse already provides a framework for launching applications, including dialogs and configuration storage. The user already expects to launch a tool or application from the launch dialog, but not from a view elsewere. Therefore, the target view should not be used for the purpose of managing launch configurations. Of course, the target view could provide shortcuts for create/run launch configurations. But of course, the target view would be only complementary to features that are already on the Eclipse launch framework.
Created attachment 98998 [details] Patch for Remote Tools Plugins
Created attachment 98999 [details] Sample extension project
Daniel, do you want me to commit this to HEAD?
Daniel, here are my thoughts on your comments: 1.Your suggestion on having only one ITargetType "Generic Host" is fine for us. I also believe performance tools can work for the ITargetElement under the "Generic Host" 2.I agree that we'd better to take Eclipse launch framework to provide the launch, configuration and so on for performance tools. However, we would like to have Remote Environment View providing some functions. The reason is that we see there are two user scenarios on running performance tools remotely: --The user is a application developer, who develops an application and launch it remotely to get result. On launching, performance tools are provided to be optionally invoked to detect application's performance. --The user is a performance analyzer, whose purpose is tunning some application. The application generally isn't developed by the user, and user expect a set of UI other than that of launch framework to perform remote data collection. From this point, I think these two cases are not duplicate. Even more, we can have Remote Environment View to list the configurations created on launch dialog and provide shortcuts to invoke the launch.
Greg, I would prefer to wait before committing the patch. The patch is still a proposal how the target environment could be enhanced. Lets wait until we have a final patch according to discussions that are still ongoing.
(In reply to comment #8) > Greg, I would prefer to wait before committing the patch. The patch is still a > proposal how the target environment could be enhanced. Lets wait until we have > a final patch according to discussions that are still ongoing. Daniel, could I know when we will have the final enhanced environment view?
Created attachment 106391 [details] Patch for Remote Tools Plugins-0703 Comparing to the last patch, there are two changes: 1. Remove the "Perf Remote Host" target type, take generic ITargetType for performance analysis. 2. Refine the extension definition, enable ITargetElement to be populated with children by contributing a class to the extension point.
Created attachment 106392 [details] Sample extension project-0703 The sample project based on the "Patch for Remote Tools Plugins-0703".
Comment on attachment 106391 [details] Patch for Remote Tools Plugins-0703 A new patch that fixes several issues will be attached.
Comment on attachment 106392 [details] Sample extension project-0703 A new example that fixes several issues will be attached.
Summary from our meeting today: A summary of patch: - Creates a new extension point that contributes a class that provides children to existing target elements. To each target element is assigned an instance of this class. The view is shows children according to information produced by this class. Major issues: a) The new extension point must work for all target types, including others like simulator or others that might be implemented by other contributors. We agree that the patch already satisfies this issue, but implementation could be better. From code I see that each target element receives a extension instance to enumerate children. It would be better to have a extension manager class that provides children for all the target elements, avoiding the target elements to have to handle with the children extension itself. b) Several (more than one) plugins must be able to contribute to the new extension point. Currently, only one plugin at time can contribute with performance tools to the Remote Targets view. From code I see that each target type refers to exactly one extension to provide children. This means that only the first contributor to the extension point is considered. In order to address these two issues, I suggest a simpler approach for the implementation: Instead changing the target element implementation to handle multiple references to class instances that enumerate children (tools and configurations), lets have a each contributor define a manager class that provides children for target elements. That means: - target element interface does not need to be changed - target element does not store references to extensions that provide children (the target element is agnostic to existence of children) - each extension point contributes a class (eg ChildrenProvider) that provides an enumeration of children (and children of children, and so on). - this class hides all implementation details of the contribution and manages the logic to create children for each target element. Advantages of my suggestion: - Code changes are restricted mostly to GUI, which uses ChildrenProvider classes (provided by extensions) to query each target element for children. - This ChildrenProvider centralized code, model and control for each extension. - Allows contributions from multiple plugins (automatically solves issue b) - Works transparently for multiple targets (automatically solves issue a). - Since ChildrenProvider for each extension, as opposed to the current patch that has one extension instance for each target element. The ChildrenProvider allows having only one entry point to notify changes in children hierarchy. There are some minor bugs in the patch: - The Remote Target view breaks correct hierarchy when adding performance tools. - All target elements display the same children, regardless for which tool/target element the configuration was created.
Created attachment 107589 [details] Patch for Remote Tools Plugins-0716 New patch based on comment #14
Created attachment 107590 [details] Sample extension project-0716
Summary of discussion on patch (the one uploaded on 2008-7-16): A summary of patch: -Contributor contributes an instance of IChildrenProvider, which provides the children of a target element, and the children of the children, and so on. The target view invoke IChildrenProvider.getChildren(Object), then contributor checks the type of object, and then return children of ITargetElement, or children of the given object. Enhancement on the patch 1. IChildrenProvider.getChildren(ITargetElement) only returns children of an ITargetElement 2. Define a specific interface (INode) for the children of the ITargetElement's children, and the children in more deeper hierarchies. This interface will define four methods: getChildren(), getParent(), getDisplayText(), getIcon(). The target view will invoke these methods in its content provider and label provider, to manage the tree hierarchy under ITargetElment's children, display text and icon for children node.
Created attachment 107845 [details] Patch for Remote Tools Plugins-0718 Patch based on comment #17.
Created attachment 107846 [details] Patch for Remote Tools Plugins-0718 Patch based on comment #17
The design used in patch #107846 looks good for me. Would it be possible to change: Object[] ChildrenProviderManager.getChildren(ITargetElement targetElement) to INode[] ChildrenProviderManager.getChildren(ITargetElement targetElement) and Object[] IChildrenProvider.getChildren(ITargetElement targetElement) to INode[] IChildrenProvider.getChildren(ITargetElement targetElement) and Object[] INode.getChildren(); to INode[] INode.getChildren(); Reason: All 3 methods return only INode elements. Therefore, the specialized interface is preferred. Also, for consistency, I would move all classes from the org.eclipse.ptp.remotetools.environment.extension package to org.eclipse.ptp.remotetools.environment.core.
Created attachment 108018 [details] Patch for Remote Tools Plugins-0722 Patch based on comment #20
Created attachment 108022 [details] Sample extension project-0722 Sample project for testing Remote Tools extension. It covers the scenarios: --There is more than one target type, each one with multiple target elements. --There is more than plugin extending the point org.eclipse.ptp.remotetools.environment.core.remoteEnvironmentControlDelegate.
Created attachment 111065 [details] Patch for Remote Tools Plugins-0827
Hi Hongchang. My comments on your patch. The changes in org.eclipse.ptp.remotetools.environment.core look good. What is the purpose of INode.doubleClickExecute()? This seems to be a UI implementation and should not be placed in the core plugin. I dont think this method should be in INode. The workload controller extension point is defined in the org.eclipse.ptp.remotetools.environment.core plugin but used exclusively in the org.eclipse.ptp.remotetools.environment.ui. I think that a) the code to read and managed the workload controller extension should go into org.eclipse.ptp.remotetools.environment.core, or b) the workload controller extension should be declared in org.eclipse.ptp.remotetools.environment.ui. Is the test plugin from comment #22 still valid? Please note that the attached jar file is missing the .project file, that would make importing the sample project much easier.
Created attachment 111953 [details] Patch for Remote Tools Plugins-0908
Created attachment 112034 [details] Patch for Remote Tools Plugins-0908 (without the icon)
Created attachment 112035 [details] Icon for TargetTypeElement in Remote Tools View
Good work! The patch is fine now. Patch committed. Thanks!
