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

Add section in workspace to handle the agents #1774

Closed
ashumilova opened this issue Jul 17, 2016 · 25 comments
Closed

Add section in workspace to handle the agents #1774

ashumilova opened this issue Jul 17, 2016 · 25 comments
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Milestone

Comments

@ashumilova
Copy link
Contributor

We will need a new section in the workspace details to handle the agents.
This is a proposed design that we will iterate.
1 - workspace details - agents - july 2016

@ashumilova ashumilova added kind/task Internal things, technical debt, and to-do tasks to be performed. team/plugin labels Jul 17, 2016
@ashumilova ashumilova added sprint/next status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it. labels Aug 26, 2016
@ashumilova ashumilova added kind/enhancement A feature request - must adhere to the feature request template. and removed kind/task Internal things, technical debt, and to-do tasks to be performed. labels Sep 6, 2016
@ibuziuk
Copy link
Member

ibuziuk commented Sep 12, 2016

@ashumilova I have started to work on this one. After browsing a little bit through REST API I figure out how to get agents per workspace:

this.agents = this.workspace.config.environments.default.machines["dev-machine"].agents;

Basically, the agents' names for my test workspace are:

org.eclipse.che.terminal
org.eclipse.che.ws-agent
org.eclipse.che.ssh

looking at the mockup I find it hard to understand how can I get agents' "state" and "description" properties. REST API for agents e.g api/agent/name/org.eclipse.che.ws-agent return object with properties like: "name" / "dependencies" / "properties" / "script". Am I on the wrong track ?

@slemeur
Copy link
Contributor

slemeur commented Sep 12, 2016

@ibuziuk : Glad, and super excited to see you contributing to Che. Please give a look at the updated mockup.

Agents will be defined per machines in environments.
1 - workspace details - environments - save changes option1

In the agent section, you'd have the ability to see the whole list of agents installed with Che. User will then just have to use the slider to activate the agents in the machine or not.

@ibuziuk
Copy link
Member

ibuziuk commented Sep 12, 2016

@slemeur hey, buddy! I also super exited having a possibility to contribute - for now I'm only getting familiar with a code base though. New mockup looks completed, however I still baffled with the agent properties "state" / "description" - I can not figure out what API should be used here.

@tolusha
Copy link
Contributor

tolusha commented Sep 13, 2016

Currently agents don't provide description.

@ashumilova
Copy link
Contributor Author

ashumilova commented Sep 13, 2016

What we have is dependencies, properties and script. @tolusha can we add description - to have smth human readable.

@ibuziuk about state - the state is determined whether machine includes the agent provided by Che or not. So we display all agents provided by Che in the list, and those that are mentioned in machines agents list - are active for this machine, others - inactive.

@slemeur
Copy link
Contributor

slemeur commented Sep 13, 2016

@tolusha : Each agents must be provided with a set of meaningful information for end-user.
When I see the name = "org.eclipse.che.terminal" and all the properties you have related to the agent, they seem perfect for internal - but definitely not to be exposed to end-users.

Please create the issue to improve this area.

@ibuziuk
Copy link
Member

ibuziuk commented Sep 13, 2016

@tolusha @ashumilova Thanks for clarifications

@ibuziuk
Copy link
Member

ibuziuk commented Sep 21, 2016

@slemeur @ashumilova As it was mentioned above Che Agent API is missing "description" property and the "name" property like "org.eclipse.che.ls.json" does not look viable from the user perspective. So, I guess with the current version of API the best layout would be the following (please, let me know if it is ok for the initial implementation):

agents

Basically, in the next iteration it would be great to change the Agents REST API the following way:

  1. Add id field. The value of it would be the current value of the name field i.e. org.eclipse.che.ls.json
  2. Amend name field so, that it would reflect meaningful name of the agent i.e. JSON language service
  3. Add description field
  4. Change REST API, so that new id will be used instead of name e.g. change GET /agent/name/{name} to GET /agent/id/{id}

WDYT ?
If you think it is relevant, I would be glad to work on it ;-)

@TylerJewell
Copy link

Yes, yes, yes, and yes. :)

I think using the fully qualified name as you have it is a reasonable first start so that we can go ahead and get the capabilities merged into master. We can then make an improvement PR once the API is updated. So I am +1 for splitting into two segments.

@ibuziuk
Copy link
Member

ibuziuk commented Sep 21, 2016

@TylerJewell awesome! I am planning to provide the initial implementation by the end of the week

@TylerJewell
Copy link

That is really awesome. There are just a couple of lingering language server and agent bugs, but if all goes well, we may have a shippable release with agents and language server by next week!

@ibuziuk
Copy link
Member

ibuziuk commented Sep 22, 2016

@ashumilova @slemeur @TylerJewell while implementing the agents enabling / disabling I have noticed that API does not allow to disable ws-agent agent:

ws-agent

Am I correct that the switch for this agent should be permanently disabled ?

@ashumilova
Copy link
Contributor Author

@ibuziuk yes we have that restriction that at least one machine in environment should be with ws-agent.
Perhaps it should be disabled in case it's the only one machine with ws-agent. @slemeur what do you think?

@slemeur
Copy link
Contributor

slemeur commented Sep 23, 2016

@ibuziuk and @ashumilova : Yes at least one machine from the environment needs the ws-agent. I think we should use the "Dev" slider to handle that.

The "ws-agent" will still be listed in the agents list, but in disable state.

@ibuziuk
Copy link
Member

ibuziuk commented Sep 23, 2016

@slemeur just want to be sure that we are on the same page:

  • ws-agent switch is listed but always disabled for machine regardless of it's state
  • switching of the ws-agent happens only via "Dev" slider

I'm polishing my PR now, but I do not really know when 5.0.0-M3 is going to be released to be sure that the fix will be ready on time for it

@ashumilova
Copy link
Contributor Author

The approximate date of 5.0.0-M3 release is Monday (09/26).

@ibuziuk
Copy link
Member

ibuziuk commented Sep 23, 2016

@ashumilova oh, awesome I was thinking it's today ;-)

@slemeur
Copy link
Contributor

slemeur commented Sep 23, 2016

@ibuziuk : yes perfect! we are on the same page ;)

@ibuziuk
Copy link
Member

ibuziuk commented Sep 23, 2016

out
@ashumilova @slemeur PR is ready - #2577
The biggest concern is that enabling / disabling agent results in screen blinking. Actually, it is also true for other sections like "Dev" / slider in "RAM" - for agents it is more noticeable though.

@ashumilova
Copy link
Contributor Author

Fixed by @ibuziuk

@ashumilova ashumilova removed sprint/next status/open-for-dev An issue has had its specification reviewed and confirmed. Waiting for an engineer to take it. labels Sep 28, 2016
@ibuziuk
Copy link
Member

ibuziuk commented Sep 28, 2016

@ashumilova @slemeur as I understand it was decided to split this issue in two parts. First one is done and merged, the second one is coupled with changing Agents REST API and adjusting front-end part for it. Should I work on the second part ?

@slemeur
Copy link
Contributor

slemeur commented Sep 28, 2016

@ibuziuk Yes sure ! It would definitely be great for the product !
We have not planned this work in the upcoming sprint for us, so if you would like to contribute it, it would be wonderful :) !

@ibuziuk
Copy link
Member

ibuziuk commented Sep 28, 2016

@slemeur great! should I create another issue for it ?

@bmicklea bmicklea added this to the 5.0.0-M3 milestone Sep 28, 2016
@slemeur
Copy link
Contributor

slemeur commented Sep 29, 2016

@ibuziuk Yes please, it would make it clearer ;)

@ibuziuk
Copy link
Member

ibuziuk commented Sep 29, 2016

@slemeur done - #2654

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

No branches or pull requests

6 participants