Skip to content
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

redis report #2289

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

redis report #2289

wants to merge 7 commits into from

Conversation

terrazoon
Copy link
Contributor

@terrazoon terrazoon commented Jan 22, 2025

Description

Add a Redis report to the platform admin area. Also, change the checks.yml so isort and black are run before file checks so style checks NEVER FAIL AGAIN when you push a PR.

Security Considerations

N/A

Copy link
Contributor

@ccostino ccostino left a comment

Choose a reason for hiding this comment

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

This is awesome to see and will be very useful, thanks @terrazoon!

I have the same note about the black/isort changes here in that we can't include them in the CI/CD pipeline like that (but we can work them in locally and just make sure folks do that!), and a quick question about the additional environment checks.

.github/workflows/checks.yml Outdated Show resolved Hide resolved
writer.writerow(["", "Max Memory", max_memory])
writer.writerow(["", "Memory Fragmentation Ratio", mem_fragmentation])
writer.writerow(["", "Memory Fragmentation Quality", frag_quality, frag_note])
#writer.writerow(["", "Memory Fragmentation Note", frag_note])
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the fragmentation note is included in the line above - can we just get rid of this one?

Suggested change
#writer.writerow(["", "Memory Fragmentation Note", frag_note])

@@ -68,11 +68,12 @@ def _get_access_token(code): # pragma: no cover
id_token = get_id_token(response_json)
nonce = id_token["nonce"]
nonce_key = f"login-nonce-{unquote(nonce)}"
stored_nonce = redis_client.get(nonce_key).decode("utf8")
if not os.getenv("NOTIFY_ENVIRONMENT") == "development":
Copy link
Contributor

Choose a reason for hiding this comment

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

I see there an additional check here and below for the environment to see that it's not development - just double checking what this is accounting for that's specific to the development environment as I'm not remembering. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I try to run it locally, it almost always fails here and I end up having to comment out the code in order to work.

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.

Platform Admin -- show redis memory usage
2 participants