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

NMS-7797: Migrate to Vaadin v8.5.2 #2224

Merged
merged 49 commits into from Nov 27, 2018
Merged

NMS-7797: Migrate to Vaadin v8.5.2 #2224

merged 49 commits into from Nov 27, 2018

Conversation

mvrueden
Copy link
Contributor

@mvrueden mvrueden commented Nov 6, 2018

Here we migrate Vaadin to 8.5.2 in order to make it later more easily compile with Java 9.

With Vaadin 8 there is a new API, which should be used instead.
However Vaadin provides so called compatibility modules which contain the old API classes, which you can detect via the import statements, as they start with com.vaadin.v7 (e.g. com.vaadin.v7.ui.TextField).
Here I migrated only the vaadin applications to make use of the old com.vaadin.v7 classes and did not upgrade to the Vaadin 8 API.

By default however, the Compatibility classes are not integrated, so a custom Widgetset must be defined. So org.opennms.vaadin.DefaultWidgetset must be defined by all Vaadin Applications which use the v7 compatibility classes.

Besides that the biggest changes are:

  • setReadOnly(...) is no longer available for non Editable fields
  • setImmediate(...) can no longer be set manually and is automatically assumed
  • UI.getCurrent(...).access(...) is now working as documented and therefore no longer working for us

@agalue, @gallenc, @christianpape I would like to invite you to verify the Vaadin Applications you originally contributed and let me know if they look and behave correctly in Vaadin 8. Changes for the Plugin Manager can be viewed here: https://github.com/OpenNMS/osgi-plugin-manager/compare/1.1-SNAPSHOT...1.2-SNAPSHOT?expand=1

This PR requires the RemotePollerMap to be removed. See PR #2207

Open Issues

@smith-opennms
Copy link
Contributor

image

@christianpape
Copy link
Contributor

The configuration page for surveillance view and the OpsBoard is currently broken. I'll fix...

@christianpape
Copy link
Contributor

Checked Surveillance View/Dashboard, Ops Board, Dashlets and BSM configuration page.

@@ -28,13 +28,13 @@

package org.opennms.features.topology.app.internal.gwt.client.ui;

import java.util.Collection;
Copy link
Contributor

Choose a reason for hiding this comment

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

We really need to get IDEA and Eclipse to agree on where java.util.* classes go. :D

Copy link
Contributor Author

@mvrueden mvrueden Nov 12, 2018

Choose a reason for hiding this comment

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

For exactly this reason, there is custom configuration you must apply in order to use IntelliJ accordingly to prevent these re-imports. See https://github.com/opennms/ide-setup and for even more details https://wiki.opennms.org/wiki/IDEA_and_OpenNMS

However, when I first used IntelliJ code were already contributed by other IntelliJ user's, not following the eclipse import guides. Therefore occasionally if the contributor is not carefully enough, you get re-imports.

That is why I always recommend git add -i -p instead of git add . to check what you are committing. In addition git checkout -p helps you getting rid of those re-imports.

@j-white
Copy link
Contributor

j-white commented Nov 14, 2018

For the ops board, there appears to be a new margin on the left:
image

@agalue
Copy link

agalue commented Nov 19, 2018

@mvrueden asked me to give it a try to verify the MIB compiler.

I've compiled CISCO-WIRELESS-IF-MIB and its dependencies. Everything went well, I was able to generate data collection and event definitions.

However, the UIs for editing/modifying events is not working. I can see an icon at the top right besides the table, but I cannot click it to see more details about it. To reproduce the problem, just install the branch, start OpenNMS, go to Admin, then "Customize Event Configurations", then choose any file from the list (Select Events Configuration File), and once the table with the event definitions is presented, choose one, and you'll see that nothing is presented below the table (which is the expected behavior). As this is not happening, I don't know if I can actually modify an event through the UI.

Nothing is shown on web.log and/or jetty-server.log, but I'm seeing the following on Karaf.log:

2018-11-19T07:54:16,775 ERROR com.vaadin.server:8.5.2(85) [qtp39117586-544] com.vaadin.server.DefaultErrorHandler:
java.lang.IllegalStateException: This component does not support the read-only mode, since state is of type CustomComponentState and does not inherit AbstractFieldState
        at com.vaadin.ui.AbstractComponent.setReadOnly(AbstractComponent.java:1055) [85:com.vaadin.server:8.5.2]
        at org.opennms.features.vaadin.events.EventForm.setReadOnly(EventForm.java:310) [277:org.opennms.features.vaadin-snmp-events-and-metrics:24.0.0.SNAPSHOT]

Same thing happens when trying to edit data collection groups (Admin -> Manage SNMP Collections and Data Collection Groups). To reproduce the problem is the same thing, there is no need to compile any MIB, just try to edit an exiting SNMP collection, or Data Collection Group.

As the implementation when clicking on a table to show the details below it is the same everywhere, I believe that after fixing one, the rest of them, can be easily fixed.

Please let me know when this part is fixed, so I can test again, and verify the pending behavior (modify something and make sure it is persisted).

@mvrueden
Copy link
Contributor Author

mvrueden commented Nov 19, 2018

Thank you @agalue for your quick feedback. I addressed the problems you were having and hopefully all should work now. Please give it a re-spin.

@agalue
Copy link

agalue commented Nov 20, 2018

Overall, editing events, SNMP collections and data collection groups seems to be working as expected after your change. Although, I made just simple edits. If you want me to perform more comprehensive checks let me know, but my time is limited, so I would do it next week if necessary.

The only weird behavior I found is when scrolling over the tabular data on edit mode. The one that gave me trouble was the RRA list. The current workaround for the RRA table is click on an item and then scroll

@mvrueden
Copy link
Contributor Author

I can confirm the issue @agalue reported. I addressed it accordingly. From my point of view this is enough testing. If any issues arise at a later point we can still address them then. Thank you for your quick feedback and responsiveness.

@mvrueden
Copy link
Contributor Author

From my point of view this is ready to be merged to develop. I talked to @gallenc and he said if it compiles it works for him. If more issues pop up, we can address them then.

@mvrueden mvrueden merged commit bfe6fca into develop Nov 27, 2018
@mvrueden mvrueden deleted the jira/NMS-7797-2018 branch November 27, 2018 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants