Skip to content

Conversation

@shaur97
Copy link

@shaur97 shaur97 commented Mar 25, 2025

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

How was this patch tested?

(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 before opening a pull request.

className="text-black m-0 breadcrumb d-flex align-items-center"
style={{ fontSize: 24 }}
>
{" "}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove redundant strings.

parseJSONData
} from "./api/Utility.ts";
import { get } from "lodash";
import AmbariAboutModal from "./AmbariAboutModal";
Copy link
Contributor

Choose a reason for hiding this comment

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

File missing.

Comment on lines +47 to +52
{showAmbariAboutModal ? (
<AmbariAboutModal
isOpen={showAmbariAboutModal}
onClose={() => setShowAmbariAboutModal(false)}
/>
) : null}
Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that isOpen alone can control the display and hiding of AmbariAboutModal. Why is an additional layer of render conditional wrapping needed?

setInLocalStorage('ambari', encryptData(JSON.stringify(data)));

const headers = {
'Authorization': 'Basic ' + 'invalid_username:password'
Copy link
Contributor

Choose a reason for hiding this comment

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

The current ambari-server uses cookie-based authentication, but it seems you’ve implemented it using a JWT approach. What is the purpose of this here?

Choose a reason for hiding this comment

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

@zRains this implementation logic seems to be as per angular js implementation of ambari-admin auth.js

Copy link

@nikita15p nikita15p left a comment

Choose a reason for hiding this comment

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

@shaur97 add JIRA ID in the commit message. CI gives error Ooops, no jira id found :(

@jojochuang jojochuang changed the title Ambari 26425 ambari admin sign out AMBARI-26425: ambari admin sign out Apr 16, 2025
@jojochuang
Copy link
Contributor

Updated PR subject and retriggered precommit tests.

Encountered rat failure: [INFO] Rat check: Summary over all files. Unapproved: 1, unknown: 1, generated: 0, approved: 181 licenses.

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.

4 participants