Skip to content

fix: Model addition, error log printing #2499

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 5, 2025
Merged

fix: Model addition, error log printing #2499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/aliyun_bai_lian_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/10/16 17:01
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -35,6 +36,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model: AliyunBaiLianEmbedding = provider.get_model(model_type, model_name, model_credential)
model.embed_query(_('Hello'))
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/aliyun_bai_lian_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 18:41
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -56,6 +57,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
for chunk in res:
print(chunk)
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/aliyun_bai_lian_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# coding=utf-8
import traceback
from typing import Dict

from langchain_core.messages import HumanMessage
Expand Down Expand Up @@ -48,6 +49,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.invoke([HumanMessage(content=gettext('Hello'))])
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/aliyun_bai_lian_model_provider/credential/reranker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/9/9 17:51
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -35,6 +36,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model: AliyunBaiLianReranker = provider.get_model(model_type, model_name, model_credential)
model.compress_documents([Document(page_content=_('Hello'))], _('Hello'))
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
3 changes: 2 additions & 1 deletion apps/setting/models_provider/impl/aliyun_bai_lian_model_provider/credential/stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# coding=utf-8

import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -30,6 +30,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential)
model.check_auth()
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your code looks mostly clean and well-organized, but there are a few potential improvements:

  1. Exception Handling: The traceback.print_exc() function is good for debugging purposes, especially when you're working remotely or in an environments where logging might not be set up properly. However, I would recommend adding more detail to the exception handling logic.

Here's one way it could look after making these changes:

        except Exception as e:
            print(f"An error occurred while checking authentication: {e}")
            
            # Detailed traceback
            traceback.print_exception(*sys.exc_info())
            
            if isinstance(e, AppApiException):
                raise e
            
            if raise_exception:

This will log both the exception message and the full stack trace, which can help in tracking down errors effectively.

  1. Consistency with Code Style:
    • Ensure consistent use of quotes (either single or double) throughout your file.
    • If you want to keep using triple quotes ("""), make sure they are appropriately formatted across the entire class/module.
  1. Security Considerations:
    • When raising exceptions in production, consider wrapping them with custom Http404, PermissionDenied, etc., rather than bare raise.

Example:

if raise_exception:
    raise Http404("Invalid model credentials")

or

if raise_exception:
    raise PermissionDenied("Insufficient permissions to access this resource")

These exceptions have HTTP status codes that align better with typical RESTful API usage patterns.

Overall, the code structure is clear and does what you intended it to do. With these minor adjustments, you'll have cleaner, more detailed error handling and potentially improved readability.

Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/aliyun_bai_lian_model_provider/credential/tti.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 18:41
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -76,6 +77,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
res = model.check_auth()
print(res)
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
3 changes: 2 additions & 1 deletion apps/setting/models_provider/impl/aliyun_bai_lian_model_provider/credential/tts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# coding=utf-8

import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -64,6 +64,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.check_auth()
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes made in this code snippet include:

  1. Adding the traceback import at the top of the file, which will print a stack trace when an exception occurs during execution.

  2. Moving the call to provider.get_model() inside a try-except block, so that any exceptions raised by this operation can be caught and reported appropriately.

This addition provides better error handling for cases where errors occur while trying to retrieve models from the provider. It's generally good practice to catch exceptions explicitly instead of leaving them to propagate all the way up the call stack, allowing you to handle specific types of errors more gracefully and maintain application stability. Additionally, printing the traceback can help developers debug issues by providing context about what went wrong during execution.

Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/anthropic_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# coding=utf-8
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -52,6 +53,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
for chunk in res:
print(chunk)
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/anthropic_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 18:32
@desc:
"""
import traceback
from typing import Dict

from langchain_core.messages import HumanMessage
Expand Down Expand Up @@ -55,6 +56,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential)
model.invoke([HumanMessage(content=gettext('Hello'))])
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/aws_bedrock_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -35,6 +36,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
except AppApiException:
raise
except Exception as e:
traceback.print_exc()
if raise_exception:
raise AppApiException(ValidCode.valid_error.value,
_('Verification failed, please check whether the parameters are correct: {error}').format(
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/aws_bedrock_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -53,6 +54,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
except AppApiException:
raise
except Exception as e:
traceback.print_exc()
if raise_exception:
raise AppApiException(ValidCode.valid_error.value,
gettext(
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/azure_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 17:08
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -35,6 +36,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential)
model.embed_query(_('Hello'))
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/azure_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# coding=utf-8
import base64
import os
import traceback
from typing import Dict

from langchain_core.messages import HumanMessage
Expand Down Expand Up @@ -55,6 +56,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
for chunk in res:
print(chunk)
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/azure_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 17:08
@desc:
"""
import traceback
from typing import Dict

from langchain_core.messages import HumanMessage
Expand Down Expand Up @@ -55,6 +56,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.invoke([HumanMessage(content=gettext('Hello'))])
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/azure_model_provider/credential/stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# coding=utf-8
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -31,6 +32,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential)
model.check_auth()
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/azure_model_provider/credential/tti.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# coding=utf-8
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -67,6 +68,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
res = model.check_auth()
print(res)
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/azure_model_provider/credential/tts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# coding=utf-8
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -49,6 +50,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.check_auth()
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/deepseek_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 17:51
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -55,6 +56,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.invoke([HumanMessage(content=gettext('Hello'))])
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/gemini_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/12 16:45
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -34,6 +35,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential)
model.embed_query(_('Hello'))
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
3 changes: 2 additions & 1 deletion apps/setting/models_provider/impl/gemini_model_provider/credential/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# coding=utf-8

import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -52,6 +52,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
for chunk in res:
print(chunk)
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code snippet appears to be written in Python using Django framework. Here are some points of review:

  1. Imports: The traceback module is imported at the beginning, which is appropriate for capturing and printing stack traces of errors.

  2. Variable Names: Variable names like model_type, model_name, model_credential, etc., suggest they might represent database table columns. However, without more context, it's hard to confirm their usage patterns.

  3. Error Handling:

    • There is an attempt to catch exceptions within a try-except block when processing results (res) after executing a query or API call.
    • After catching an exception, if it's an instance of AppApiException, the exception is re-raised.
    • If raise_exception=True (not shown here), then the function would return early if the inner exception was caught.

  4. Traceback Printing:

    • traceback.print_exc() is called unconditionally when an exception occurs during querying or API calls. This can help in understanding what went wrong but may also clutter the stdout if logging is configured elsewhere.
    • Consider using a more localized logger instead of relying on print statements for error handling.

  5. Code Structure:

    • While not overly complex, the code structure follows typical functional programming idioms used in Django apps.

  6. Potential Issues:

    • Ensure that if there are any exceptions due to missing data or invalid credentials, those scenarios should be gracefully handled rather than just raising them unless raise_exception=True.
    • Verify that all operations involve parameterized queries or sanitization to prevent SQL injection attacks unless explicitly allowed (e.g., through Django ORM methods).

  7. Optimization Suggestions:

    • Ensure that no redundant calculations or unnecessary loops are present.
    • Look into optimizing I/O-bound operations by caching results where feasible, depending on the application's requirements.

Overall, the code looks well-structured with proper error handling in place. It would benefit from additional comments and possibly adjustments to improve readability and maintainability.

Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/gemini_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 17:57
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -56,6 +57,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
res = model.invoke([HumanMessage(content=gettext('Hello'))])
print(res)
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/gemini_model_provider/credential/stt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# coding=utf-8
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -29,6 +30,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential)
model.check_auth()
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/kimi_model_provider/credential/llm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 18:06
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -55,6 +56,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential, **model_params)
model.invoke([HumanMessage(content=gettext('Hello'))])
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/local_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/11 11:06
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext_lazy as _, gettext
Expand Down Expand Up @@ -34,6 +35,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model: LocalEmbedding = provider.get_model(model_type, model_name, model_credential)
model.embed_query(gettext('Hello'))
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/local_model_provider/credential/reranker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/9/3 14:33
@desc:
"""
import traceback
from typing import Dict

from langchain_core.documents import Document
Expand Down Expand Up @@ -35,6 +36,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model: LocalBaseReranker = provider.get_model(model_type, model_name, model_credential)
model.compress_documents([Document(page_content=gettext('Hello'))], gettext('Hello'))
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
2 changes: 2 additions & 0 deletions apps/setting/models_provider/impl/openai_model_provider/credential/embedding.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
@date：2024/7/12 16:45
@desc:
"""
import traceback
from typing import Dict

from django.utils.translation import gettext as _
Expand Down Expand Up @@ -34,6 +35,7 @@ def is_valid(self, model_type: str, model_name, model_credential: Dict[str, obje
model = provider.get_model(model_type, model_name, model_credential)
model.embed_query(_('Hello'))
except Exception as e:
traceback.print_exc()
if isinstance(e, AppApiException):
raise e
if raise_exception:
Expand Down
Loading