Skip to content

[Core] Suppress SAWarning for CasbinRule#6796

Merged
DanielZhangQD merged 3 commits intomasterfrom
2951
Aug 21, 2025
Merged

[Core] Suppress SAWarning for CasbinRule#6796
DanielZhangQD merged 3 commits intomasterfrom
2951

Conversation

@DanielZhangQD
Copy link
Copy Markdown
Collaborator

@DanielZhangQD DanielZhangQD commented Aug 21, 2025

Suppress SAWarning for CasbinRule. Fix #6772

Tested (run the relevant ones):

  • Code formatting: install pre-commit (auto-check on commit) or bash format.sh
  • Any manual or new tests for this PR (please specify below)
    • With sqlalchemy_adapter 1.3.0 and 1.6.0
      • Checks that the sky commands do not report warning /usr/local/lib/python3.10/site-packages/sqlalchemy_adapter/adapter.py:33: SAWarning: This declarative base already contains a class with the same class name and module name as sqlalchemy_adapter.adapter.CasbinRule, and will be replaced in the string-lookup table. after the fix.
      • Launch a cluster successfully (workspace permission works)
      • Create a new user and new records are inserted into casbin_rule
  • All smoke tests: /smoke-test (CI) or pytest tests/test_smoke.py (local)
  • Relevant individual tests: /smoke-test -k test_name (CI) or pytest tests/test_smoke.py::test_name (local)
  • Backward compatibility: /quicktest-core (CI) or pytest tests/smoke_tests/test_backward_compat.py (local)

@DanielZhangQD DanielZhangQD marked this pull request as ready for review August 21, 2025 03:26
Comment thread sky/users/permission.py Outdated
Comment on lines +26 to +27
# Suppress SQLAlchemy warnings about duplicate class names in
# sqlalchemy_adapter
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why does it raise this warning at the first place?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

sqlalchemy_adapter v1.4.0 introduced a change in apache/casbin-python-sqlalchemy-adapter#4, where if no data_class is set when initialize Adapter, it will create a new data class with the same name CasbinRule, which is same with the default one.
I updated the PR to use the default CasbinRule when initialize Adapter.

Copy link
Copy Markdown
Collaborator

@aylei aylei left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@DanielZhangQD DanielZhangQD merged commit fc24837 into master Aug 21, 2025
16 checks passed
@DanielZhangQD DanielZhangQD deleted the 2951 branch August 21, 2025 07:29
massaindustries pushed a commit to Seeweb/skypilot that referenced this pull request Aug 26, 2025
* fix sawarning for casbin_rule

* use default casbin rule class

* update ut
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.

CLI get warning message after restart the API server with pg enabled

2 participants