Skip to content

Mistral large model support & error handling fix#52

Merged
hari-kuriakose merged 20 commits into
mainfrom
fix/file_upload_security
Mar 5, 2024
Merged

Mistral large model support & error handling fix#52
hari-kuriakose merged 20 commits into
mainfrom
fix/file_upload_security

Conversation

@gaya3-vijayakumar

@gaya3-vijayakumar gaya3-vijayakumar commented Mar 4, 2024

Copy link
Copy Markdown
Contributor

What

  • Code review comments and mypy issues resolution for file upload security PR
  • Change exception handling to report original error in case the error response is not JSON

Why

  • Exception while handling error message is masking the actual error.

How

  • Handle any exception occurring during error processing

Relevant Docs

Related Issues or PRs

Dependencies Versions / Env Variables

  • unstract-sdk - 0.14.0
  • unstract-adapters - 0.4.0

Notes on Testing

...

Screenshots

image

Checklist

I have read and understood the Contribution Guidelines.

Comment thread .pre-commit-config.yaml
Comment thread backend/utils/FileValidator.py
Comment thread backend/utils/FileValidator.py
Comment thread backend/backend/exceptions.py Outdated

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please address my error handling concerns around testing adapters.

NIT: Also there's duplication of logic between the backend and SDK. The SDK does perform these validations for tools in a worfklow and I understand we're doing this validation for prompt studio - we could try to refactor and reuse logic wherever possible

Comment thread backend/backend/exceptions.py Outdated
Comment thread backend/adapter_processor/views.py
Comment thread backend/adapter_processor/views.py Outdated

@athul-rs athul-rs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@chandrasekharan-zipstack @muhammad-ali-e Please take a look into file validator file before merging, I have minimum context here.
Rest LGTM.

@gaya3-vijayakumar gaya3-vijayakumar changed the title Fix/file upload security Mistral large model support Mar 4, 2024
@gaya3-vijayakumar gaya3-vijayakumar changed the title Mistral large model support Mistral large model support & error handling fix Mar 4, 2024

@hari-kuriakose hari-kuriakose left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@gaya3-zipstack LGTM overall.

Added few comments though.

Comment thread .pre-commit-config.yaml
Comment thread backend/adapter_processor/exceptions.py
Comment thread backend/backend/exceptions.py
Comment thread backend/adapter_processor/adapter_processor.py Outdated
@hari-kuriakose
hari-kuriakose merged commit 31977a5 into main Mar 5, 2024
@hari-kuriakose
hari-kuriakose deleted the fix/file_upload_security branch March 5, 2024 07:46
mohamed-siddhiq pushed a commit that referenced this pull request Mar 7, 2024
* Add file upload validation

* Code formatting

* Change values to constants

* Set file pointer to the beginning of the file

* Change exception handling to report adapter error properly, Code review fix for FileValidation.py

* Revert special JSON handling

* Version upgrade of adapters & sdk. Update .gitignore to exclude local dependency files

* Updating SDK version for added support of large models in Mistral

---------

Co-authored-by: Hari John Kuriakose <hari@zipstack.com>
pk-zipstack pushed a commit that referenced this pull request Aug 20, 2025
* Add file upload validation

* Code formatting

* Change values to constants

* Set file pointer to the beginning of the file

* Change exception handling to report adapter error properly, Code review fix for FileValidation.py

* Revert special JSON handling

* Version upgrade of adapters & sdk. Update .gitignore to exclude local dependency files

* Updating SDK version for added support of large models in Mistral

---------

Co-authored-by: Hari John Kuriakose <hari@zipstack.com>
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.

4 participants