Skip to content

[ZEPPELIN-6064] Change default web UI to new UI #4802

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

Merged
merged 20 commits into from
Sep 3, 2024

Conversation

tbonelee
Copy link
Contributor

What is this PR for?

For several years, Zeppelin has supported both the old and new UIs, with the old UI remaining as the default.
However, as the new UI offers a more modern design, I believe it might be beneficial to consider making it the default interface.

Rationale

The new UI presents a more contemporary user experience, which could align better with users' expectations today.
Transitioning to the new UI as the default may help in keeping Zeppelin more up-to-date and user-friendly.
Of course, I am aware that this change might impact various parts of the project, so I propose we carefully review this before proceeding.

What type of PR is it?

Improvement

Todos

  • - Update all references to the web app context paths.

What is the Jira issue?

How should this be tested?

  • Verify that the new UI is accessible via root path.
  • Ensure the old UI can still be accessed via /classic.

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No, but it may affect users' accustomed access paths.
  • Does this needs documentation? Maybe it would be helpful to provide guidance on how users could access the old UI under the new path, especially for those accustomed to the previous default. However, I'm not entirely sure if this is the best approach, so I haven't added any documentation for now.

@tbonelee
Copy link
Contributor Author

@jongyoul I've made a PR to set the new UI as the default. Please review it when you have some time.

@kevinjmh
Copy link
Member

I would like to switch to the new UI too, but tests and bugs fixed should be ahead.

@kevinjmh
Copy link
Member

image
I tried several options above, short-cut does not work, clone only create an empty paragraph without content

@jongyoul
Copy link
Member

By the way, I assume we can deploy two versions of Apache Zeppelin with current and new UI in some ways like setting parameters when packaging them and so on. However, your change supports only running Zeppelin with the new UI. Do you have any idea? :-)

Copy link
Contributor

@Reamer Reamer left a comment

Choose a reason for hiding this comment

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

I know that the new user interface has some bugs, but even the old user interface is not free of errors. Unfortunately, the technology of the old user interface is also very outdated, making further development difficult.
I would be in favor of changing the default user interface, perhaps this will encourage a few frontend developers to correct the bugs in this user interface.

By the way, I assume we can deploy two versions of Apache Zeppelin with current and new UI in some ways like setting parameters when packaging them and so on. However, your change supports only running Zeppelin with the new UI. Do you have any idea? :-)

We have the 'web-angular' profile, which builds the new user interface. This should definitely be switched on by default when the default interface is changed.
As far as I can see, this change does not result in running Zeppelin with the new UI.

@jongyoul
Copy link
Member

We have the 'web-angular' profile, which builds the new user interface. This should definitely be switched on by default when the default interface is changed.
As far as I can see, this change does not result in running Zeppelin with the new UI.

Yes, correct. Moreover, even if we include the profile, we still need to change some server code as ZeppelinServer has a hardcoded configuration for the current and new UI.

By the way, for the improvement of the new UI, deploying the new UI version would be helpful.

@pan3793
Copy link
Member

pan3793 commented Aug 26, 2024

Strongly +1 for making the Angular UI as default even though there are some bugs need to be fixed, and renaming the existing one to "Classic UI".

@tbonelee
Copy link
Contributor Author

Additionally, I made the following changes:

  1. Added the web-classic profile, removed the web-angular profile, and changed the default UI included during the build to the new UI (formerly included in web-angular profile).
  2. Renamed configuration variable names:
    • zeppelin.war -> zeppelin.classic.war
    • zeppelin.angular.war -> zeppelin.war
  3. Renamed module names:
    • zeppelin-web -> zeppelin-web-classic
    • zeppelin-web-angular -> zeppelin-web

Copy link
Contributor

@Reamer Reamer left a comment

Choose a reason for hiding this comment

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

Please revert the change regarding renaming. This leads to an unnecessary number of changes. I think the profile change in pom.xml makes sense.

@tbonelee
Copy link
Contributor Author

@Reamer
I have reverted the commits related to 2. and 3.
Is there anything else that needs to be reverted?

@pan3793
Copy link
Member

pan3793 commented Aug 28, 2024

code changes lgtm, I have not tested it locally though.

@Reamer
Copy link
Contributor

Reamer commented Aug 28, 2024

The code changes are now much clearer. @tbonelee Please perform a rebase to the current master branch, as there are merge conflicts. Furthermore, I think the Selenium tests for the “Classic” UI need to be adjusted.

@jongyoul
Copy link
Member

I have a question. In this PR, will we permanently change the default web to a new one? I intended to release two versions with an old and new UI separately. After releasing 0.12.0 with those manners we can try to change the default UI for the 0.13.0 release in my understanding. WDYT?

@pan3793
Copy link
Member

pan3793 commented Aug 28, 2024

@jongyoul The current release bundles both classic and new UI, and allows users to switch to each other by clicking a button, the proposed approach "change new UI to default UI" looks straightforward to me, and users still have the option to switch back to the classic UI by just clicking a button if they find bugs in the new UI.

@jongyoul
Copy link
Member

@jongyoul The current release bundles both classic and new UI, and allows users to switch to each other by clicking a button, the proposed approach "change new UI to default UI" looks straightforward to me, and users still have the option to switch back to the classic UI by just clicking a button if they find bugs in the new UI.

Yes, right. we can switch for now but I think it would be better to change gradually so my idea is to provide a new version with a separate package like zeppelin-bin-new-ui.tgz and so on for the next release. Then, we can check the community's feedback and change the default permanently.

@Reamer
Copy link
Contributor

Reamer commented Aug 28, 2024

I would have liked more feedback from the community and improvements with the current solution. Unfortunately, there was only sporadic feedback.
I see the change of user interfaces as uncritical, especially as it is easy to switch between the two.
In my opinion, a separate release makes the release process and implementation unnecessarily complicated.

@jongyoul
Copy link
Member

Yes, I hope so. For the release project, we can edit the release script to release two versions but we still need to change some code in ZeppelinServer and so on.

@tbonelee
Copy link
Contributor Author

After rebasing, I updated the URLs for the Selenium tests, and I also updated the test URLs in DirAccessTest

@Reamer
Copy link
Contributor

Reamer commented Aug 29, 2024

@tbonelee Please check the CI

@tbonelee tbonelee requested a review from Reamer August 29, 2024 18:28
@tbonelee
Copy link
Contributor Author

tbonelee commented Aug 29, 2024

@Reamer The Selenium test passed in my personal repository's GitHub Actions. Therefore I assumed it was a temporary issue and did not make any change.
I've tried to fix the remaining broken tests. Please review and confirm.

Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

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

I have tested this patch locally

  • Switch to Classic/Default UI button works well
  • Successfully run simple Spark jobs with both classic and default UIs

image image

@jongyoul
Copy link
Member

jongyoul commented Sep 2, 2024

Thank you for checking it. By the way, I'm still worried about providing the new UI as a default for the next release but let's see how's going.

I suggest that we provide an option to store the type of UI, which means we should use the same type of UI after we restart the Zeppelin. Could you please add the configuration? Otherwise, could you please create a ticket for it?

@tbonelee
Copy link
Contributor Author

tbonelee commented Sep 2, 2024

@jongyoul
Are you suggesting that when someone enters the top-level URL of Zeppelin, they should be redirected to the most recently accessed UI? Or did you have sth else in mind?
In any case, I think there might be additional aspects we need to discuss, so it might be a good idea to create a new issue ticket.
I'll go ahead and register a new issue.

@pan3793
Copy link
Member

pan3793 commented Sep 2, 2024

when someone enters the top-level URL of Zeppelin, they should be redirected to the most recently accessed UI?

Hmm... maybe we just need a configuration to allow the administrator to decide the default UI?

@tbonelee
Copy link
Contributor Author

tbonelee commented Sep 2, 2024

I've made a new issue regarding UI type persistence feature.
I believe there are some points that need further discussion. Please feel free to comment on the issue.
https://issues.apache.org/jira/browse/ZEPPELIN-6074

@jongyoul
Copy link
Member

jongyoul commented Sep 2, 2024

As Cheng said, I thought we would have a configuration storing the type of UI and the Zeppelin server uses it. But, we don't have that kind of configuration yet so we'd better discuss it.

Copy link
Contributor

@Reamer Reamer left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you for your contribution.
As long as no further comments are received, I will merge this change into the master on Wednesday.

@pan3793
Copy link
Member

pan3793 commented Sep 3, 2024

As all comments are addressed, merging to master

@pan3793 pan3793 merged commit 7708b71 into apache:master Sep 3, 2024
28 checks passed
Reamer added a commit to Reamer/zeppelin that referenced this pull request Sep 5, 2024
Reamer added a commit to Reamer/zeppelin that referenced this pull request Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants