Skip to content

Fire ProjectCreatedEvent on importProject#9954

Merged
vparfonov merged 2 commits intomasterfrom
che#9924
Jun 12, 2018
Merged

Fire ProjectCreatedEvent on importProject#9954
vparfonov merged 2 commits intomasterfrom
che#9924

Conversation

@vparfonov
Copy link
Copy Markdown
Contributor

@vparfonov vparfonov commented Jun 6, 2018

Signed-off-by: Vitalii Parfonov vparfonov@redhat.com

What does this PR do?

Fire ProjectCreatedEvent on importProject and during setting ProjectConfiguration firstly

What issues does this PR fix or reference?

#9924

Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>
@vparfonov vparfonov requested a review from dkulieshov June 6, 2018 12:19
@vparfonov
Copy link
Copy Markdown
Contributor Author

ci-test

@benoitf benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Jun 6, 2018
@codenvy-ci
Copy link
Copy Markdown

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:9954
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

Copy link
Copy Markdown
Contributor

@tsmaeder tsmaeder left a comment

Choose a reason for hiding this comment

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

This PR does not cover the case of importZip, which is used for Selenium tests.

@vparfonov
Copy link
Copy Markdown
Contributor Author

@tsmaeder In case importZip we don't create Project in ProjectRegistry so fire event in this case no reason

@dkulieshov
Copy link
Copy Markdown

@tsmaeder

This PR does not cover the case of importZip, which is used for Selenium tests.

It is expected behavior, according to this comment, specifically

In case of just creating folder in the projects root - configuration is not assigned explicitly => Project even is not fired.

@tsmaeder
Copy link
Copy Markdown
Contributor

tsmaeder commented Jun 7, 2018

Yes, but a project appears in the navigator without a ProjectCreatedEvent ever being sent. Without the event being sent, I cannot tell jdt.ls to look at the project.
Perhaps just listening to file system events is the way to go for jdt.ls and to completely ignore che projects, but then we'll get into situations where what che thinks is a project and what jdt.ls thinks is a project will be different. The will be no fun to debug.

@vparfonov
Copy link
Copy Markdown
Contributor Author

ok, got it. Looks like you are right

@dkulieshov
Copy link
Copy Markdown

@tsmaeder I think I understand what you mean. Just to be sure that we're on the same wave. We consider that the project is created only when corresponding configuration is added to project config registry. In other words when a folder is created/archive is unpacked in the projects' root we don't consider it as a project until its configuration is added.

For the case that you mentioned (selenium tests) they basically do two things: they unzip a folder and add a project configuration via updateProject API call. So you're right that the problem exists, however the problem is not that importZip API call does not publish an event, the problem is that when we add a project config with updateProject API call we do not publish project created event.

So, eventually there are two bugs:

  • no ProjectCreatedEvent on project import
  • no ProjectCreatedEvent on project update, when we add new configuration

I also think that your suggestion to move project related events on a separate abstraction level to avoid event micromanagement and all related errors sounds good.

Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>
@vparfonov vparfonov merged commit b799f4c into master Jun 12, 2018
@vparfonov vparfonov deleted the che#9924 branch June 12, 2018 15:07
@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 Jun 12, 2018
@benoitf benoitf added this to the 6.7.0 milestone Jun 12, 2018
hbhargav pushed a commit to hbhargav/che that referenced this pull request Dec 5, 2018
* Fire ProjectCreatedEvent on importProject

Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>

* Fire ProjectCreatedEvent on setting project configuration firtsly

Signed-off-by: Vitalii Parfonov <vparfonov@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Outline of a bug - must adhere to the bug report template.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants