Skip to content

Fix: remove deprecated functions - #19449

Open
Lynn-Inf wants to merge 2 commits into
infiniflow:mainfrom
Lynn-Inf:fix/rm-deprecated-llm-service-code
Open

Fix: remove deprecated functions#19449
Lynn-Inf wants to merge 2 commits into
infiniflow:mainfrom
Lynn-Inf:fix/rm-deprecated-llm-service-code

Conversation

@Lynn-Inf

@Lynn-Inf Lynn-Inf commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

As title.

@Lynn-Inf
Lynn-Inf requested a review from wangq8 September 9, 2026 09:21
@Lynn-Inf Lynn-Inf added the ci Continue Integration label Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 429382c8-3642-4471-89f4-9ce2e4fbf060

📥 Commits

Reviewing files that changed from the base of the PR and between c2bdea5 and f997ad2.

📒 Files selected for processing (1)
  • api/db/joint_services/tenant_model_service.py

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

TenantLLMService removes obsolete configuration, lookup, usage, provisioning, deletion, and model-type methods. Unused imports are removed. Tenant lookups in tenant_model_service.py now import TenantService from user_service.

Changes

Tenant LLM service reduction

Layer / File(s) Summary
Service imports and core boundary
api/db/services/tenant_llm_service.py
Unused imports and API-key, model lookup, authorization, embedding fallback, and metadata methods are removed.
Usage and provisioning method removal
api/db/services/tenant_llm_service.py, api/db/joint_services/tenant_model_service.py
Usage accounting, OpenAI model listing, environment-driven model provisioning, tenant deletion, and model-type resolution methods are removed. TenantService is imported from user_service for tenant lookups.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to f997a

This change removes deprecated TenantLLMService functionality and redirects retained tenant lookups to the centralized service without an identified behavior regression. The change is ready to merge.

Suggested reviewers: euvre

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description includes the required Summary heading but only states “As title.” It does not explain the purpose, background, or scope of removing the deprecated functions. Replace “As title.” with a brief summary that explains which deprecated functions were removed, where they were removed, and why the cleanup was required.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: removing deprecated functions. It is concise and relevant to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI

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.

❤️ Share

A rabbit trims the service tree,
And clears old paths from memory.
The models hop, the tenants race,
With cleaner imports in their place.
Unused methods fade from view,
While runtime work stays true.

Comment @coderabbitai help to get the list of available commands.

@wangq8 wangq8 left a comment

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.

This is an AI review comment.

Thanks for the cleanup. Most of the large deletion looks safe (the OCR ensure_*_from_env helpers have been relocated to api/db/joint_services/tenant_model_service.py, and all runtime callers already import them from there). However, there is one blocking issue.

🔴 Blocking: removed TenantService re-export is still required

This PR removes:

from api.db.services.user_service import TenantService

from api/db/services/tenant_llm_service.py. But api/db/joint_services/tenant_model_service.py still imports it from this module:

# api/db/joint_services/tenant_model_service.py:36
from api.db.services.tenant_llm_service import TenantService

and uses it at lines 166, 344 and 446 (TenantService.get_by_id, TenantService.get_joined_tenants_by_user_id).

Because this PR only touches tenant_llm_service.py and does not update tenant_model_service.py, dropping the re-export will cause ImportError: cannot import name 'TenantService' from 'api.db.services.tenant_llm_service' as soon as that module is imported — and it is imported at startup by api/apps/services/models_api_service.py, rag/app/naive.py, rag/flow/parser/parser.py, etc. This would break the application.

Suggested fix (either one):

  • Update tenant_model_service.py to from api.db.services.user_service import TenantService, or
  • Keep the TenantService import in tenant_llm_service.py.

✅ Verified safe

  • ensure_mineru_from_env / ensure_paddleocr_from_env / ensure_opendataloader_from_env: now defined in tenant_model_service.py; all runtime callers already import from there.
  • get_api_key, get_model_config, get_my_llms, increase_usage, increase_usage_by_id, get_openai_models, llm_id2llm_type, split_model_name_and_factory, _decode_api_key_config, _encode_api_key_config: no remaining runtime callers (only commented-out code and unit-test stubs/monkeypatches reference them).
  • Kept model_instance and LLM4Tenant do not reference any of the removed imports (os, json, settings, MINERU_*, PADDLEOCR_*, OPENDATALOADER_*, IntegrityError).

Minor (non-blocking)

  • tools/scripts/mysql_migration.py (~line 550) still has a comment referencing _encode_api_key_config; it is now stale and could be updated for clarity.

This is an AI review comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continue Integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants