You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by "JiaLiangC (via GitHub)" <gi...@apache.org> on 2024/10/12 02:48:16 UTC

[PR] Ambari-26147: Add Ruff integration to ambari [ambari]

JiaLiangC opened a new pull request, #3844:
URL: https://github.com/apache/ambari/pull/3844

   ## What changes were proposed in this pull request?
   The entire Ambari project has accumulated a large amount of Python code, which is the result of the collective efforts of multiple contributors to make Ambari better. 
   
   | Language    | Files | Lines  | Code   | Comments | Blanks |
   |-------------|-------|--------|--------|----------|--------|
   | Java        | 3330  | 690679 | 427116 | 156389   | 107174 |
   | JavaScript  | 2125  | 512742 | 374238 | 87081    | 51423  |
   | Plain Text  | 68    | 500082 | 0      | 497409   | 2673   |
   | JSON        | 758   | 479988 | 479689 | 0        | 299    |
   | Python      | 1096  | 218057 | 175499 | 9547     | 33011  |
   | XML         | 1135  | 156581 | 130300 | 20189    | 6092   |
   
   
   During this process, a problem has emerged: everyone has different coding styles, which leads to inconsistencies in the submitted code. This can result in various inconveniences, such as increased understanding costs of the underlying principles and greater difficulty in identifying the root causes of issues. As the codebase grows, these problems become increasingly apparent. Therefore, we are introducing [ruff](https://github.com/astral-sh/ruff), along with some additional processes, to format the code and alleviate these issues.
   
   What is Ruff?
   
   An extremely fast Python linter and code formatter, written in Rust. Its speed is one of its advantages. More information can be found here: [ruff](https://github.com/astral-sh/ruff).
   (Please fill in changes proposed in this fix)
   
   ## How was this patch tested?
   manual test CI/CD
   After formatting thousands of files with Ruff, I deployed a simple cluster to verify that Ambari runs smoothly.
   
   ![image](https://github.com/user-attachments/assets/d7a97444-3847-42d4-bae2-4778952c3618)
   
   (Please explain how this patch was tested. Ex: unit tests, manual tests)
   (If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)
   
   Please review [Ambari Contributing Guide](https://cwiki.apache.org/confluence/display/AMBARI/How+to+Contribute) before opening a pull request.


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "arshadmohammad (via GitHub)" <gi...@apache.org>.
arshadmohammad commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2421647950

   Hi @JiaLiangC,
   Which environment are you deploying the cluster for testing? It seems you're having success with the setup.
   I recently tried to build and deploy an Ambari cluster on Rocky Linux 8 and encountered two blocking issues:
   [AMBARI-26183](https://issues.apache.org/jira/browse/AMBARI-26183) (Fixed)
   [AMBARI-26187](https://issues.apache.org/jira/browse/AMBARI-26187) (Still open)
   
   
   


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "vishalsuvagia (via GitHub)" <gi...@apache.org>.
vishalsuvagia commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2436825176

   Hi @JiaLiangC , many changes include indentation / spacing between the lines, the current codebase uses two spaces for indentation / spacing instead of the typical four spaces / tabs. Is it possible to configure ruff to consider two spaces as the indentation and spacing size as it is the standard followed. Doing so may help reduce the size of the change as well and have clear diff for the actual changes.


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2408652871

   Wow, this is huge and yet it seems worth having. Skimmed through at high level, looks good. I wonder if anyone else is also interested to review this, otherwise let's get this in and see how things go.
   
   Nice work!


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2421864363

   @arshadmohammad  Good catch! Your deployment failed because of this PR: https://github.com/apache/ambari/pull/3770, which introduced numerous bugs in the frontend. 
   Although the subsequent PR https://github.com/apache/ambari/pull/3846 fixed many issues, around 60 of them, there are still many bugs remaining. One of them is the blocking issue you encountered, which is a bug caused by the frontend upgrade. I tested using the code from before this PR was merged: AMBARI-25289 Upgrade Jquery and Bootstrap to latest versions #3770.


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2423454480

   
   > Hi @JiaLiangC, Which environment are you deploying the cluster for testing? It seems you're having success with the setup. I recently tried to build and deploy an Ambari cluster on Rocky Linux 8 and encountered two blocking issues: [AMBARI-26183](https://issues.apache.org/jira/browse/AMBARI-26183) (Fixed) [AMBARI-26187](https://issues.apache.org/jira/browse/AMBARI-26187) (Still open)
   
   @arshadmohammad Already resolved by this pr  https://github.com/apache/ambari/pull/3849.


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2426116171

   @arshadmohammad I have added the usage of Ruff involved in this PR to the description. Additionally, all code changes in this PR are the result of ruff format.


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2408445302

   @virajjasani Could you help check this PR?


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC closed pull request #3844: Ambari-26147: Add Ruff integration to ambari
URL: https://github.com/apache/ambari/pull/3844


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "arshadmohammad (via GitHub)" <gi...@apache.org>.
arshadmohammad commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2423620598

   Thanks @JiaLiangC, I will review this patch


-- 
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-26147: Add Ruff integration to ambari [ambari]

Posted by "JiaLiangC (via GitHub)" <gi...@apache.org>.
JiaLiangC commented on PR #3844:
URL: https://github.com/apache/ambari/pull/3844#issuecomment-2437340389

   @vishalsuvagia Thank you for the reminder. I've changed the indent-width to 2 and committed it. This indeed makes it easier to review. However, there are still a massive 900 files, which I believe makes manual review very difficult. We can only rely on unit tests and subsequent bug reports after usage. Currently, the unit tests have passed, and I've deployed a complete cluster in my environment without finding any issues so far. Theoretically, code formatting tools like ruff shouldn't change the code semantics.
   


-- 
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