refactor(cloud-native): avoid using f-string in logger message#13383
Open
refactor(cloud-native): avoid using f-string in logger message#13383
Conversation
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (6)
📝 WalkthroughWalkthroughConverts many f-string logger invocations to parameterized logging across numerous scripts and modules, removes unused imports and the internal extract_common_libs helpers from several mod_context.py files, and deletes the WebDAV sync script plus a commented reference in an entrypoint. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mo-auto
added
comp-jans-pycloudlib
kind-enhancement
Member
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
mo-auto
added
comp-docker-jans-auth-server
comp-docker-jans-casa
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
jans-pycloudlib/jans/pycloudlib/config/google_config.py (1)
224-224:⚠️ Potential issue | 🟡 MinorInconsistent logging format: still uses
.format()instead of %-style.This line was not updated to use parameterized logging like the rest of the file. For consistency with the PR's goal of enabling lazy evaluation, this should also be converted to %-style formatting.
🔧 Proposed fix
- logger.info("Added secret version: {}".format(response.name)) + logger.info("Added secret version: %s", response.name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jans-pycloudlib/jans/pycloudlib/config/google_config.py` at line 224, The logging call uses str.format instead of parameterized %-style logging; update the logger.info("Added secret version: {}".format(response.name)) to use logger.info("Added secret version: %s", response.name) (locate the call in google_config.py where response.name is logged, e.g., inside the function that adds a secret version) so the log becomes lazily-evaluated and consistent with the rest of the file.
♻️ Duplicate comments (1)
docker-jans-casa/scripts/mod_context.py (1)
34-34:⚠️ Potential issue | 🔴 Critical
exec_cmdis called with an invalid signature (runtime failure).Line 34 passes three positional arguments, but
exec_cmdtakes a single command string. This will fail before download runs.Proposed fix
- out, err, code = exec_cmd("wget -q %s -O %s", download_url, dist_file) + out, err, code = exec_cmd(f"wget -q {download_url} -O {dist_file}")#!/bin/bash # Verify exec_cmd signature and this call site. rg -nP '^def exec_cmd\(cmd: str\)' jans-pycloudlib/jans/pycloudlib/utils.py -C3 rg -nP 'exec_cmd\(' docker-jans-casa/scripts/mod_context.py -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-jans-casa/scripts/mod_context.py` at line 34, The call site in mod_context.py uses exec_cmd("wget -q %s -O %s", download_url, dist_file) but exec_cmd accepts a single command string; change the call to pass a single formatted command string (e.g., build the command with f-strings or %-formatting) so exec_cmd receives one argument; update the call referencing exec_cmd in mod_context.py (the wget download call) to something like exec_cmd(f"wget -q {download_url} -O {dist_file}") or the equivalent string formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-jans-casa/scripts/bootstrap.py`:
- Around line 169-173: Replace the overly broad Exception handler around
Path.unlink with a filesystem-specific one: catch OSError (and its subclasses)
instead of Exception in the try/except that calls
lock_path.unlink(missing_ok=True); update the except block that currently logs
via logger.warning("Unable to remove %s; reason=%s", lock_file, exc) to catch
OSError as exc so only filesystem errors are handled while allowing other
exceptions to surface.
In `@docker-jans-casa/scripts/mod_context.py`:
- Around line 45-46: The error fallback is inverted: change the fallback
assignment so the actual command output is preserved (set err = err or out) and
keep the logger.error call using err.decode(); update the assignment where out
and err are handled (the variables out, err) so logger.error("Unable to extract
%s; reason=%s", dist_file, err.decode()) logs the correct output for dist_file.
In `@docker-jans-cloudtools/scripts/certmanager.py`:
- Around line 444-445: The log calls are using the stale variable name because
`name` is set after logging; before calling logger.info and
self.meta_client.exec_cmd you must resolve the container's name for the current
`container` (e.g., call whatever resolver is used elsewhere in this module to
get the container name for that `container`) and then use that resolved `name`
in the log and command. Update both occurrences around the restore loop (the
logger.info that references `name` and the subsequent exec via
`self.meta_client.exec_cmd(container, ...)`) so `name` is assigned from the
current `container` first, then log and execute using that `name` and
`container`.
- Line 421: Replace the incorrect brace-style format in the logger call: change
the logger.info invocation that currently reads logger.info("creating backup of
{name}:{jks_fn}", name, jks_fn) to use the logging module's printf-style
formatting and positional args (i.e. "creating backup of %s:%s", name, jks_fn)
so the logger.info call with variables name and jks_fn matches the surrounding
lines and won't raise a formatting error.
In `@docker-jans-cloudtools/scripts/cleaner.py`:
- Around line 45-47: Replace the broad "except Exception as exc" in the table
cleanup loop with an explicit catch for database errors: import SQLAlchemyError
from sqlalchemy.exc and change the handler to "except SQLAlchemyError as exc" so
only DB-layer issues are logged with logger.warning("Unable to cleanup expired
entries in %s; reason=%s", table, exc); let other exceptions propagate (do not
swallow them) so programming errors surface. Ensure the import and the exception
symbol (SQLAlchemyError), the logger variable, and the table loop location
(where the current except sits) are updated accordingly.
In `@docker-jans-fido2/scripts/mod_context.py`:
- Around line 45-46: The error log uses a different variable than the fallback
assignment: after the line "out = out or err" ensure the logger.error call uses
that same fallback (out) instead of err (i.e., change the logger.error call that
currently uses err.decode() to use out.decode() or otherwise reference the "out"
variable), so the actual extracted output is logged consistently for dist_file.
In `@docker-jans-link/scripts/mod_context.py`:
- Around line 45-46: The log for extraction failures is using err.decode() even
though the code normalizes the process output into the variable out; update the
logger.error call that references dist_file to log the normalized buffer
(out.decode()) instead of err.decode() so any captured stderr/stdout merged into
out is preserved; locate the logger.error(...) near the out = out or err
assignment in mod_context.py and replace the error payload to use out (decoded)
and keep the existing message format.
In `@docker-jans-scim/scripts/mod_context.py`:
- Around line 44-47: The log currently prints err.decode() after assigning out =
out or err, which can report the wrong buffer; update the block around the
failing-extraction handling so the decoded value logged matches the variable
used—either change the assignment to err = out or err (to match the existing
logger.error(err.decode())) or keep out = out or err and log out.decode()
instead; ensure this fix is applied in the failing-extract branch that
references dist_file, out, err and logger.error.
In `@jans-pycloudlib/jans/pycloudlib/config/google_config.py`:
- Line 104: Remove the leftover commented debug line containing the logger call
in the GoogleConfig code (the commented "logger.info(...
self.google_secret_name, self.version_id)" line); delete that commented-out
statement from the class/method where it appears (references:
self.google_secret_name, self.version_id, logger.info) so the codebase is clean,
unless you intentionally need the log—in which case uncomment and ensure the
variables are available and formatted correctly.
In `@jans-pycloudlib/jans/pycloudlib/persistence/sql.py`:
- Around line 1239-1241: The warning handler that catches ProgrammingError is
indexing exc.args[1] which can raise IndexError if the exception lacks that
element; update the logger call in the except block that references
ProgrammingError (the block containing logger.warning("Unable to detect
JSON...")) to avoid direct indexing—log the full exception or a safe fallback
(e.g., use str(exc) or check len(exc.args) before accessing exc.args[1]) so the
warning always prints a reason without risking IndexError.
In `@jans-pycloudlib/jans/pycloudlib/secret/aws_secret.py`:
- Around line 217-220: Fix the typo in the logger.warning message in
aws_secret.py: change "splitted" to "split" in the parameterized log string used
in the logger.warning call (the one that logs data_length,
self.max_payload_size, parts) so the message reads "... It will be split into %s
parts."; keep the existing parameterized logging style for lazy evaluation.
In `@jans-pycloudlib/jans/pycloudlib/secret/google_secret.py`:
- Around line 271-273: Update the multipart warning message in google_secret.py
(the logger.warning call that logs data_length, self.max_payload_size, parts) to
use clearer wording: replace "is exceeding" with "exceeds" and "splitted" with
"split", and tighten phrasing (e.g., "The secret payload size %s bytes exceeds
the maximum of %s bytes; it will be split into %s parts."). Keep the same format
specifiers and variables (data_length, self.max_payload_size, parts) and call
site (logger.warning) unchanged.
---
Outside diff comments:
In `@jans-pycloudlib/jans/pycloudlib/config/google_config.py`:
- Line 224: The logging call uses str.format instead of parameterized %-style
logging; update the logger.info("Added secret version:
{}".format(response.name)) to use logger.info("Added secret version: %s",
response.name) (locate the call in google_config.py where response.name is
logged, e.g., inside the function that adds a secret version) so the log becomes
lazily-evaluated and consistent with the rest of the file.
---
Duplicate comments:
In `@docker-jans-casa/scripts/mod_context.py`:
- Line 34: The call site in mod_context.py uses exec_cmd("wget -q %s -O %s",
download_url, dist_file) but exec_cmd accepts a single command string; change
the call to pass a single formatted command string (e.g., build the command with
f-strings or %-formatting) so exec_cmd receives one argument; update the call
referencing exec_cmd in mod_context.py (the wget download call) to something
like exec_cmd(f"wget -q {download_url} -O {dist_file}") or the equivalent string
formatting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ef2edacf-dd14-49a9-9581-8af55f209e80
📒 Files selected for processing (36)
docker-jans-auth-server/scripts/auth_conf.pydocker-jans-auth-server/scripts/bootstrap.pydocker-jans-auth-server/scripts/jks_sync.pydocker-jans-auth-server/scripts/lock.pydocker-jans-auth-server/scripts/mod_context.pydocker-jans-casa/scripts/auth_conf.pydocker-jans-casa/scripts/bootstrap.pydocker-jans-casa/scripts/entrypoint.shdocker-jans-casa/scripts/jca_sync.pydocker-jans-casa/scripts/mod_context.pydocker-jans-casa/scripts/upgrade.pydocker-jans-cloudtools/scripts/certmanager.pydocker-jans-cloudtools/scripts/cleaner.pydocker-jans-config-api/scripts/bootstrap.pydocker-jans-config-api/scripts/mod_context.pydocker-jans-config-api/scripts/plugins.pydocker-jans-configurator/scripts/bootstrap.pydocker-jans-fido2/scripts/bootstrap.pydocker-jans-fido2/scripts/mod_context.pydocker-jans-link/scripts/bootstrap.pydocker-jans-link/scripts/mod_context.pydocker-jans-persistence-loader/scripts/sql_setup.pydocker-jans-scim/scripts/bootstrap.pydocker-jans-scim/scripts/mod_context.pyjans-pycloudlib/jans/pycloudlib/config/file_config.pyjans-pycloudlib/jans/pycloudlib/config/google_config.pyjans-pycloudlib/jans/pycloudlib/lock/__init__.pyjans-pycloudlib/jans/pycloudlib/manager.pyjans-pycloudlib/jans/pycloudlib/meta/kubernetes_meta.pyjans-pycloudlib/jans/pycloudlib/persistence/sql.pyjans-pycloudlib/jans/pycloudlib/secret/aws_secret.pyjans-pycloudlib/jans/pycloudlib/secret/file_secret.pyjans-pycloudlib/jans/pycloudlib/secret/google_secret.pyjans-pycloudlib/jans/pycloudlib/secret/vault_secret.pyjans-pycloudlib/jans/pycloudlib/utils.pyjans-pycloudlib/jans/pycloudlib/wait.py
💤 Files with no reviewable changes (2)
- docker-jans-casa/scripts/entrypoint.sh
- docker-jans-casa/scripts/jca_sync.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docker-jans-casa/scripts/mod_context.py (1)
45-46:⚠️ Potential issue | 🟡 MinorFix extract error fallback variable before logging.
Line 45 updates
out, but Line 46 logserr, so fallback output can be lost. Update the fallback target toerrin this block.Proposed fix
- if code != 0: - out = out or err - logger.error("Unable to extract %s; reason=%s", dist_file, err.decode()) + if code != 0: + err = err or out + logger.error("Unable to extract %s; reason=%s", dist_file, err.decode())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-jans-casa/scripts/mod_context.py` around lines 45 - 46, The code sets the fallback with out = out or err but then logs err, losing the fallback; update the logger call in the extract/error handling to log out (e.g. use out.decode()) instead of err.decode() so the fallback output is preserved when reporting the failure for dist_file via logger.error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docker-jans-casa/scripts/mod_context.py`:
- Around line 45-46: The code sets the fallback with out = out or err but then
logs err, losing the fallback; update the logger call in the extract/error
handling to log out (e.g. use out.decode()) instead of err.decode() so the
fallback output is preserved when reporting the failure for dist_file via
logger.error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c873dcae-9733-4cea-ba30-314b9c4c3b56
📒 Files selected for processing (1)
docker-jans-casa/scripts/mod_context.py
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docker-jans-link/scripts/mod_context.py (1)
45-46:⚠️ Potential issue | 🟡 MinorInconsistent error-buffer assignment breaks logging output.
Line 45 uses
out = err or out, but line 46 logserr.decode(). Iferris empty andoutcontains the failure message, the log will display an empty reason. This deviates from the canonical pattern used in line 37 and indocker-jans-auth-server/scripts/mod_context.py(see context snippet), where the assignment iserr = err or out.,
Proposed fix to align with canonical pattern
if code != 0: - out = err or out + err = err or out logger.error("Unable to extract %s; reason=%s", dist_file, err.decode()) sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-jans-link/scripts/mod_context.py` around lines 45 - 46, The log call uses err.decode() but the preceding assignment is incorrect (out = err or out), so when err is empty the reason logged is blank; change the assignment to mirror the canonical pattern by setting err = err or out before the logger.error call so the logger.error("Unable to extract %s; reason=%s", dist_file, err.decode()) will always show the non-empty error buffer (refer to variables err, out and the logger.error call in mod_context.py).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker-jans-fido2/scripts/mod_context.py`:
- Around line 37-38: The error logging currently calls err.decode() directly
which can raise if err is None or contains undecodable bytes; update the logging
to safely handle None and decoding errors by using a safe buffer and decode with
errors='replace' (e.g. use (err or out or b'').decode('utf-8',
errors='replace')) before passing to logger.error, and apply the same change for
the second occurrence that references err and out; target the block where
variables err, out, basename and logger are used (the download/error reporting
code in mod_context.py).
In `@jans-pycloudlib/jans/pycloudlib/config/google_config.py`:
- Around line 151-153: Compute safe_value(all_) once into a local variable
(e.g., payload) inside the affected methods (set and set_all), use
sys.getsizeof(payload) for the size log, and pass that local variable to
add_secret_version; this avoids evaluating safe_value(all_) twice and prevents
eager payload serialization inside logger calls by logging payload size from the
precomputed variable and then calling self.add_secret_version(payload).
In `@jans-pycloudlib/jans/pycloudlib/secret/google_secret.py`:
- Line 84: The code currently assigns a hardcoded fallback passphrase ("secret")
to self.passphrase from CN_GOOGLE_SECRET_MANAGER_PASSPHRASE and suppresses the
security finding; remove the insecure default and NOSONAR by making the env var
required: retrieve CN_GOOGLE_SECRET_MANAGER_PASSPHRASE into self.passphrase and
if it's missing or empty, raise a clear exception (or abort initialization) so
callers must provide a secure passphrase (refer to self.passphrase and
CN_GOOGLE_SECRET_MANAGER_PASSPHRASE in the GoogleSecret/initializer code).
---
Duplicate comments:
In `@docker-jans-link/scripts/mod_context.py`:
- Around line 45-46: The log call uses err.decode() but the preceding assignment
is incorrect (out = err or out), so when err is empty the reason logged is
blank; change the assignment to mirror the canonical pattern by setting err =
err or out before the logger.error call so the logger.error("Unable to extract
%s; reason=%s", dist_file, err.decode()) will always show the non-empty error
buffer (refer to variables err, out and the logger.error call in
mod_context.py).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b2f6b86-771b-4fd9-8088-1cb0277fb0fb
📒 Files selected for processing (13)
docker-jans-auth-server/scripts/mod_context.pydocker-jans-casa/scripts/bootstrap.pydocker-jans-casa/scripts/mod_context.pydocker-jans-cloudtools/scripts/certmanager.pydocker-jans-cloudtools/scripts/cleaner.pydocker-jans-config-api/scripts/mod_context.pydocker-jans-fido2/scripts/mod_context.pydocker-jans-link/scripts/mod_context.pydocker-jans-scim/scripts/mod_context.pyjans-pycloudlib/jans/pycloudlib/config/google_config.pyjans-pycloudlib/jans/pycloudlib/persistence/sql.pyjans-pycloudlib/jans/pycloudlib/secret/aws_secret.pyjans-pycloudlib/jans/pycloudlib/secret/google_secret.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
docker-jans-link/scripts/mod_context.py (1)
37-38:⚠️ Potential issue | 🟡 MinorNormalize failure text before logging to avoid losing diagnostics.
At Line 37 and Line 45,
err = err or outprefers stderr and can drop useful stdout context; directdecode()is also brittle for malformed bytes. Build a normalizedreasononce and log that.Proposed fix
- err = err or out - logger.error("Unable to download %s; reason=%s", basename, err.decode()) + reason = (out or err or b"").decode(errors="replace") + logger.error("Unable to download %s; reason=%s", basename, reason) sys.exit(1) @@ - err = err or out - logger.error("Unable to extract %s; reason=%s", dist_file, err.decode()) + reason = (out or err or b"").decode(errors="replace") + logger.error("Unable to extract %s; reason=%s", dist_file, reason) sys.exit(1)Also applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker-jans-link/scripts/mod_context.py` around lines 37 - 38, The log currently prefers stderr via "err = err or out" and decodes bytes directly, which can lose stdout context or crash on malformed bytes; update the logging to build a normalized reason string once (e.g., choose combined output like reason_bytes = err or out or b""), decode with errors='replace' and strip, and ensure it's a str before calling logger.error("Unable to download %s; reason=%s", basename, reason); apply the same change to the second occurrence around lines handling err/out so both logger.error sites use the normalized reason variable.jans-pycloudlib/jans/pycloudlib/config/google_config.py (1)
150-152:⚠️ Potential issue | 🟠 MajorAvoid duplicate
safe_value(all_)work and incorrect payload-size metric.Line 151/152 and Line 177/178 still evaluate
safe_value(all_)twice per call path, andsys.getsizeof(...)is not the serialized payload byte size. This reintroduces avoidable work and logs a misleading size.♻️ Proposed fix
@@ - logger.info('Adding/updating key %s to google secret manager', key) - logger.info('Size of secret payload : %s bytes', sys.getsizeof(safe_value(all_))) - return self.add_secret_version(safe_value(all_)) + logger.info("Adding/updating key %s to google secret manager", key) + payload = safe_value(all_) + if logger.isEnabledFor(logging.INFO): + payload_size = len(payload.encode("UTF-8")) if isinstance(payload, str) else len(payload) + logger.info("Size of secret payload: %s bytes", payload_size) + return self.add_secret_version(payload) @@ - logger.info('Size of secret payload : %s bytes', sys.getsizeof(safe_value(all_))) - return self.add_secret_version(safe_value(all_)) + payload = safe_value(all_) + if logger.isEnabledFor(logging.INFO): + payload_size = len(payload.encode("UTF-8")) if isinstance(payload, str) else len(payload) + logger.info("Size of secret payload: %s bytes", payload_size) + return self.add_secret_version(payload)#!/bin/bash set -euo pipefail FILE="jans-pycloudlib/jans/pycloudlib/config/google_config.py" echo "Inspecting set/set_all blocks..." nl -ba "$FILE" | sed -n '145,182p' echo echo "Finding repeated safe_value(all_) and sys.getsizeof usage..." rg -nP 'safe_value\(all_\)|sys\.getsizeof\(' "$FILE"Expected result: current code shows duplicate
safe_value(all_)evaluations in both methods andsys.getsizeof(...)usage for size logging.Also applies to: 177-178
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jans-pycloudlib/jans/pycloudlib/config/google_config.py` around lines 150 - 152, Compute the serialized secret payload once into a local variable and reuse it for both logging and the call to add_secret_version instead of calling safe_value(all_) twice; additionally replace sys.getsizeof(...) with the actual serialized byte length (e.g., len(payload_bytes)) when logging the "Size of secret payload" so the logged size reflects the true payload size; update the code paths that call safe_value(all_) (the set/set_all flow that then calls add_secret_version) to use this single payload variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docker-jans-link/scripts/mod_context.py`:
- Around line 37-38: The log currently prefers stderr via "err = err or out" and
decodes bytes directly, which can lose stdout context or crash on malformed
bytes; update the logging to build a normalized reason string once (e.g., choose
combined output like reason_bytes = err or out or b""), decode with
errors='replace' and strip, and ensure it's a str before calling
logger.error("Unable to download %s; reason=%s", basename, reason); apply the
same change to the second occurrence around lines handling err/out so both
logger.error sites use the normalized reason variable.
In `@jans-pycloudlib/jans/pycloudlib/config/google_config.py`:
- Around line 150-152: Compute the serialized secret payload once into a local
variable and reuse it for both logging and the call to add_secret_version
instead of calling safe_value(all_) twice; additionally replace
sys.getsizeof(...) with the actual serialized byte length (e.g.,
len(payload_bytes)) when logging the "Size of secret payload" so the logged size
reflects the true payload size; update the code paths that call safe_value(all_)
(the set/set_all flow that then calls add_secret_version) to use this single
payload variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0f6b47a-67fe-4c20-bd6f-c14464cc6bdf
📒 Files selected for processing (2)
docker-jans-link/scripts/mod_context.pyjans-pycloudlib/jans/pycloudlib/config/google_config.py
Signed-off-by: iromli <isman.firmansyah@gmail.com>
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prepare
Description
Target issue
closes #13369
Implementation Details
Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.Summary by CodeRabbit