odpi / egeria

Egeria core
https://egeria-project.org
Apache License 2.0
807 stars 261 forks source link

Polymer UI issues in 2.10 #5241

Closed planetf1 closed 3 years ago

planetf1 commented 3 years ago

In 2.10 testing I noticed that when navigating to the UI for the first time (via nginx, in the odpi-egeria-lab help deployment) I very briefly get a graphical 'forbidden' page, before being redirected to login.

This is minor, but slightly offputting. It's not to easily capture a screenshot as very brief

Is this intentional? Should something be added to clarify in the docs?

I also notice a strange UI layout after login (chrome 91.0.4472.77)

Screenshot 2021-05-28 at 10 27 44

After a search for 'csv' I get:

Screenshot 2021-05-28 at 10 23 06

The static UI content being used is 2.8.0 as per https://github.com/odpi/egeria/issues/5211

A search for 'Week' (after running the Building a data catalog notebook) returned reasonable results.

Only asset catalog is available so unable to test any other aspects.

sarbull commented 3 years ago

pleaase reopen this issue under odpi/egeria-ui

planetf1 commented 3 years ago

We can transfer to the other repo if that's useful. (click on 'move issue' on RHS, let me know if still needed)

However my understanding was that this relates to application.properties? (which is in this repo)

If so the location in the helm charts where we set environment variables (the easiest quick way to override app properties I think) is in open-metadata-resources/open-metadata-deployment/charts/odpi-egeria-lab/templates/egeria-ui.yaml & also in open-metadata-resources/open-metadata-deployment/compose/tutorials/egeria-tutorial.yaml (you’ll see where the env is set). Then they need testing with k8s (can use microk8s) and docker-compose. (also in this repo)

If changes are required, we also need to add any relevant pointer in the release notes (which are in this repo)

lpalashevski commented 3 years ago

I agree this might be still related to default configuration properties in ui-chassis-spring. We need to investigate further. @sarbull no need to close this, if we have specific finding for egeria-ui we will open related issue.

planetf1 commented 3 years ago

note that we wish to ship egeria very soon (next few days), so a short term mitigation for the current release may be needed (which could even just mean clarification in the readme of what to ignore - after all it is just the coco environment, and similar text for anyone using the UI independently since the ui-chassis is part of base egeria currently)

sarbull commented 3 years ago

@planetf1 the only things that are missing are some configuration parameters for title and description

here is an example

title = 'Open Metadata Platform:|Find the right data with governance. (dev)'
description = 'Have a question?||Please contact contact@company.com and we will answer it as soon as possible.@@What is Open Metadata?||Start your customer journey with this link to know what is Open Metadata and how can you use this applicaiton.@@Have more cool ideas?||Feel free to let us know your ideas and business challenges so we can make it better.'

@planetf1 i'm still late with the release version bump though

planetf1 commented 3 years ago

We require changes in the base, not the static UI content

I will try to do the properties update (can you tell me their exact names) and see if that works in the charts/compose I will add a note to the readme accordingly (let me know if you'll update with something better!) & a rough note in the application properties

planetf1 commented 3 years ago

I recomment opening up a further issue/discussion for any post 2.10 followup activities and/or egeria-ui specifics

planetf1 commented 3 years ago

I have modified the helm charts to inject a file 'application.properties' into the container.

However it is unclear where this file should go. I have placed into /var/www but this does not seem to have any effect ie:

$ kubectl exec -it lab-odpi-egeria-lab-uistatic-76d57d5888-75rg6 -- /bin/sh       [16:57:02]
# cd /var/www
# ls -la
total 56
drwxr-xr-x. 1 root root 4096 May 28 15:53 .
drwxr-xr-x. 1 root root 4096 May 27 08:30 ..
-rw-r--r--. 1 root root  169 May 27 08:29 about.json
-rw-r--r--. 1 root root  481 May 28 15:53 application.properties
drwxr-xr-x. 3 root root 4096 May 27 08:30 images
-rw-r--r--. 1 root root 1692 May 27 08:29 index.html
drwxr-xr-x. 2 root root 4096 May 27 08:30 locales
-rw-r--r--. 1 root root  466 May 27 08:25 manifest.json
drwxr-xr-x. 2 root root 4096 May 27 08:30 new
drwxr-xr-x. 6 root root 4096 May 27 08:30 node_modules
-rw-r--r--. 1 root root  188 May 27 08:25 service-worker.js
drwxr-xr-x. 3 root root 4096 May 27 08:30 themes
# cat application.properties
# SPDX-License-Identifier: Apache-2.0
# Copyright Contributors to the Egeria project.
title = 'Open Metadata Platform:|Find the right data with governance. (dev)'
description = 'Have a question?||Visit https://github.com/odpi/egeria .@@What is Open Metadata?||Start your customer journey with this link to know what is Open Metadata and how can you use this applicaiton.@@Have more cool ideas?||Feel free to let us know your ideas and business challenges so we can make it better.'#

with the result being the same UI issue

I then reviewed the source at https://github.com/odpi/egeria-ui/blob/74696b552562c2ccc99127fe1ac6d6b5cd7500b1/new/egeria-home.component.js & it looks as if the current code is in the 'new' subdir. I moved the file into /var/www/new/application.properties but this did not appear to work either Also added a new line - no difference

(not looked at docker compose yet)

planetf1 commented 3 years ago

See https://github.com/odpi/egeria/pull/5236 for PR which both sets the versions as needed for the release, and then adds application.properties. Docker config is still needed, but I need confirmation as to where this 'application.properties' should really go @sarbull @lpalashevski @bogdan-sava

sarbull commented 3 years ago

See #5236 for PR which both sets the versions as needed for the release, and then adds application.properties. Docker config is still needed, but I need confirmation as to where this 'application.properties' should really go @sarbull @lpalashevski @bogdan-sava

it should go in the ui-chassis's application.properties as @lpalashevski mentioned above..

lpalashevski commented 3 years ago

See #5236 for PR which both sets the versions as needed for the release, and then adds application.properties. Docker config is still needed, but I need confirmation as to where this 'application.properties' should really go @sarbull @lpalashevski @bogdan-sava

Applogies, this is incomplete implementation. We should this on master today, test it and let you know so you can cherry pick the fix in #5236. No need to configure ui-chassis-spring spearately for the chars/compose deployments but get these as default configuration like it was until now for the other app properties.

planetf1 commented 3 years ago

Thanks. I've tested release 2.10 in k8s with the fix and it looks good now.