Add readable access-control setup logs and clean up deprecation/test noise#257
Open
minhpham1810 wants to merge 3 commits intoelixir-cloud-aai:devfrom
Open
Add readable access-control setup logs and clean up deprecation/test noise#257minhpham1810 wants to merge 3 commits intoelixir-cloud-aai:devfrom
minhpham1810 wants to merge 3 commits intoelixir-cloud-aai:devfrom
Conversation
Add support for a MONGO_URI environment variable that, when set, is used as the complete MongoDB connection string, taking precedence over individual component variables (MONGO_HOST, MONGO_PORT, MONGO_USERNAME, MONGO_PASSWORD, MONGO_DBNAME). This enables advanced connection features such as replica sets, SRV records, TLS options, and URIs from external secret managers. A warning is logged when MONGO_URI is set alongside individual variables to help users avoid configuration confusion. Full backward compatibility is maintained.
- add INFO log after casbin enforcer registration in register_access_control.py - add INFO log after permission spec registration in register_access_control.py - add INFO log after access-control registration in foca.py - add focused log-flow unit test in test_register_access_control.py - update access-control tests to assert emitted logs via stderr capture in test_foca.py - replace deprecated importlib.resources path usage with files/as_file in config.py and register_access_control.py - suppress known third-party deprecation noise in pytest config in setup.cfg Suggested PR title Add readable access-control setup logs and clean up deprecation/test noise Suggested PR description Summary This PR implements issue elixir-cloud-aai#250 by adding readable confirmation logs for the access-control setup flow, then updates tests and warning handling so CI output is reliable and low-noise. What changed 1. Access-control setup logging - Added log confirming enforcer registration in register_access_control.py. - Added log confirming permission spec registration in register_access_control.py. - Added top-level log confirming access-control registration completion in foca.py. 2. Test updates - Added a focused unit test to validate setup-step logs in test_register_access_control.py. - Updated access-control assertions in test_foca.py to use stderr capture, matching current project logging behavior. 3. Deprecation cleanup - Replaced deprecated importlib.resources path usage with files/as_file in: - config.py - register_access_control.py - Added pytest warning filters for known external deprecations in setup.cfg. Why - Improves operator visibility during bootstrap by clearly signaling successful access-control setup steps. - Prevents flaky/false-negative log assertions caused by logger capture differences. - Reduces warning noise from dependency internals so real regressions are easier to spot. Validation - Ran: pytest -q test_foca.py - Result: 15 passed Issue - Closes elixir-cloud-aai#250
Reviewer's GuideAdds explicit environment-variable-driven MongoDB URI support and access-control setup logging, updates tests to validate the new behavior and log outputs, and modernizes resource loading and pytest warning handling. Sequence diagram for access-control registration and loggingsequenceDiagram
participant Foca as Foca_create_app
participant App as Flask_app
participant AC as register_access_control
participant Enf as register_enforcer
participant Perm as register_permission_specs
participant Log as logger
Foca->>AC: register_access_control(app, mongo_config, access_control_config)
AC->>Enf: register_enforcer(app, mongo_config, access_control_config)
Enf-->>AC: enforcer_registered_app
AC->>Log: info Access control enforcer registered.
AC->>Perm: register_permission_specs(app, access_control_config)
Perm->>Perm: Resolve API spec path
Perm-->>AC: permission_specs_registered_app
AC->>Log: info Access control permission specifications registered.
AC-->>Foca: access_control_configured_app
Foca->>Log: info Access control registered.
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
minhpham1810
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="foca/database/register_mongodb.py" line_range="134-143" />
<code_context>
Returns:
MongoDB client for Flask application instance.
"""
+ mongo_uri = os.environ.get('MONGO_URI')
+ if mongo_uri is not None and mongo_uri != "":
+ individual_vars = {
+ 'MONGO_HOST': os.environ.get('MONGO_HOST'),
+ 'MONGO_PORT': os.environ.get('MONGO_PORT'),
+ 'MONGO_USERNAME': os.environ.get('MONGO_USERNAME'),
+ 'MONGO_PASSWORD': os.environ.get('MONGO_PASSWORD'),
+ 'MONGO_DBNAME': os.environ.get('MONGO_DBNAME'),
+ }
+ ignored = [name for name, val in individual_vars.items() if val is not None]
+ if ignored:
+ logger.warning(
</code_context>
<issue_to_address>
**issue:** Align empty-string handling for env vars with the rest of the function when computing ignored variables.
In the new `MONGO_URI` branch, `ignored` is computed with `if val is not None`, so env vars set to `""` are still treated as set and logged as ignored. Elsewhere in this function, empty strings are treated as unset. For consistency and to avoid noisy warnings for empty env vars, use a truthiness check (`if val`) or match the existing predicate (`val is not None and val != ""`).
</issue_to_address>
### Comment 2
<location path="tests/security/access_control/test_register_access_control.py" line_range="118-127" />
<code_context>
+def test_register_access_control_logs_setup_steps(monkeypatch, caplog):
</code_context>
<issue_to_address>
**suggestion (testing):** Scope log capture to the access-control logger for more robust log assertions.
To avoid capturing unrelated INFO logs and making the test brittle, use `caplog.at_level(logging.INFO, logger="foca.security.access_control.register_access_control")` (or the correct module path) and/or assert against `caplog.records` filtered by logger name rather than relying on `caplog.text`. This keeps the test focused on the access-control logger only.
Suggested implementation:
```python
def test_register_access_control_logs_setup_steps(monkeypatch, caplog):
"""Test setup logs for access control registration flow."""
logger_name = "foca.security.access_control.register_access_control"
with caplog.at_level(logging.INFO, logger=logger_name):
access_control = AccessControlConfig(**ACCESS_CONTROL_CONFIG)
mongo_config = MongoConfig(**MONGO_CONFIG)
dummy_app = SimpleNamespace(
app=SimpleNamespace(config=SimpleNamespace(foca=SimpleNamespace(db=None)))
)
monkeypatch.setattr(
```
1. Ensure `import logging` is present at the top of `tests/security/access_control/test_register_access_control.py`. If it is missing, add `import logging` alongside the other imports.
2. Update any log assertions later in `test_register_access_control_logs_setup_steps` that currently rely on `caplog.text` to instead filter `caplog.records` by `record.name == logger_name`, e.g.:
```python
records = [r for r in caplog.records if r.name == logger_name]
assert any("expected message fragment" in r.getMessage() for r in records)
```
This will fully scope the assertions to the access-control logger and avoid brittleness from unrelated INFO logs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…rol logging tests
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
1 participant
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.
Description
Summary
This PR implements issue #250 by adding readable confirmation logs for the access-control setup flow, then updates tests and warning handling so CI output is reliable and low-noise.
What changed
Why
Validation
Fixes #250 (issue)
Type of change
Please delete options that are not relevant.
Checklist:
Summary by Sourcery
Document and support MongoDB configuration via a MONGO_URI environment variable while improving access-control setup logging and keeping tests and warnings aligned with the updated behavior.
New Features:
Enhancements:
CI:
Documentation:
Tests: