You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by "vanshuhassija (via GitHub)" <gi...@apache.org> on 2024/11/06 22:02:40 UTC
[PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
vanshuhassija opened a new pull request, #3870:
URL: https://github.com/apache/ambari/pull/3870
## What changes were proposed in this pull request?
Create a switchable experience between existing Angular version of ambari admin and newly proposed React version of Ambari Admin. The url mapping is 1:1 allowing the user to test the changes being made in application. The feature development for React project is going to be progressive, through this PR a basic layout is setup for the application.
## How was this patch tested?
Manual Testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "zRains (via GitHub)" <gi...@apache.org>.
zRains commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2464107245
@vanshuhassija I believe this approach isn't ideal. The issues from the jQuery upgrade are still being addressed, and merging WIP code and files again will complicate troubleshooting. While I understand there are some isolation measures, this fundamentally undermines the long-term integrity of the main branch. It's suited for minor enhancements and fixes, but modular changes like these are precisely what a separate branch should handle.
Regarding contributions, I think prioritizing the creation of a roadmap and sharing it with the community is a better choice. Submitting WIP code to main branch feels irresponsible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "vanshuhassija (via GitHub)" <gi...@apache.org>.
vanshuhassija commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2461500131
https://github.com/user-attachments/assets/2ceb283a-8bb4-44c2-83a4-b6182fa739b6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "vanshuhassija (via GitHub)" <gi...@apache.org>.
vanshuhassija commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2464114315
@zRains makes sense. Will raise the progress in a new branch. While there exist the tickets for the required features mapped to the parent feature, a detailed roadmap will be shared as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "zRains (via GitHub)" <gi...@apache.org>.
zRains commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2463843160
Great changes! This could be the start of a new stage for web updates. I applied your code and tried running it locally, but I encountered a few issues:
1. There's an unnecessary vite.config.js file, and the static asset entry isn't correctly configured in vite.config.ts.
2. tsconfig.node.json file no found.
3. The Bootstrap styles aren't compatible with the current Sass loader, leading to numerous warnings in the console.
<img width="1007" alt="image" src="https://github.com/user-attachments/assets/6817cd6c-1880-43e4-8c02-d7d0727acc77">
4. Some React-related errors appear after launching.
<img width="599" alt="image" src="https://github.com/user-attachments/assets/90c605de-ba32-41ae-9b5d-39b9c7234717">
6. There are missing test cases and service request logic.
IMO, these issues might take considerable time to resolve. Additionally, there are still some problems with the previous jQuery upgrade.
- [x] #3770
Therefore, I think it would be better to develop this upgrade on a new branch. Thanks for all your hard work, and I hope we can complete this change together:P
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "vanshuhassija (via GitHub)" <gi...@apache.org>.
vanshuhassija commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2461489145
@JiaLiangC The change does not impact the existing functionality in any way except the packaging for ambari-admin. The packaging changes have been tested thoroughly.
With this change a parallel new implementation of admin view is implemented and in no way and under no circumstances, changes will be required in the current Angular implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2464219122
@vanshuhassija Hello, I have created a branch for the Ambari frontend refactoring called `frontend-refactor`. Could you please submit the PR to this branch?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on code in PR #3870:
URL: https://github.com/apache/ambari/pull/3870#discussion_r1832330638
##########
ambari-admin/src/main/resources/ui/ambari-admin/src/layout/SideItemList.tsx:
##########
@@ -35,7 +35,7 @@ export enum SideItemLabels {
USERS = "Users",
VIEWS = "Views",
}
-// START GENAI@CHATGPT4
Review Comment:
I recommend using Claude 3.5 20241022, which is currently the best code model.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2461549429
@zRains WDYT？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2461152548
@vanshuhassija Thank you very much for your contribution. It is recommended that major changes be developed in a separate feature branch. If the changes can be completed before the release, they can be merged. Otherwise, due to the large scope of the changes and the relatively small number of active contributors in the community, it may impact the release of the next version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "vanshuhassija (via GitHub)" <gi...@apache.org>.
vanshuhassija commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2463954500
@zRains thanks for the review. I will be addressing these issues shortly. While I understand the issues with Jquery upgrade, this feature in no way impacts the existing functionality and does not impact the current written code. Intention is to develop the modernized version of the application (mostly written in React) while keeping the current version intact. As and when stable, we will be able to remove the previously written versions of the modules and make the modernized one as the default version.
Having a separate branch for the same would mean development in isolation which is not the case. To leverage the contributions, we can consider having the changes in trunk branch where the community can see the functionality to switch and contribute according to their capacity.
Let me know your thoughts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "vanshuhassija (via GitHub)" <gi...@apache.org>.
vanshuhassija commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2464273242
Closing this PR. A new one #3874 is raised for `frontend-refactor` branch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "vanshuhassija (via GitHub)" <gi...@apache.org>.
vanshuhassija closed pull request #3870: AMBARI-26172-Ambari-admin-react-implementation-layouting
URL: https://github.com/apache/ambari/pull/3870
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org
Re: [PR] AMBARI-26172-Ambari-admin-react-implementation-layouting [ambari]
Posted by "vanshuhassija (via GitHub)" <gi...@apache.org>.
vanshuhassija commented on PR #3870:
URL: https://github.com/apache/ambari/pull/3870#issuecomment-2464267325
Sure @JiaLiangC . Will do that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ambari.apache.org
For additional commands, e-mail: dev-help@ambari.apache.org