Skip to content
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.

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

changes in worker and tools related to sdk filestorage #844

Merged
merged 12 commits into from
Nov 22, 2024

Conversation

muhammad-ali-e
Copy link
Contributor

What

  • Implemented changes in the worker (or Runner) and tools to integrate the latest SDK feature: Remote FileStorage.

Why

  • To ensure compatibility between the tools and the Runner with the latest SDK, enabling seamless integration of the Remote FileStorage feature.

How

  • Made the Runner and tools compatible with the latest SDK by incorporating a feature flag in the Runner and conditional logic in the tools.
  • In tools, used hasattr(self, "workflow_filestorage") for backward compatibility, as the upcoming SDK version will introduce the workflow_filestorage property in the BaseTool class.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • No. Backward compatibility is maintained through the feature flag and conditional checks to ensure older versions of the SDK continue to function correctly.

Database Migrations

Env Config

Relevant Docs

Related Issues or PRs

Dependencies Versions

Notes on Testing

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

Copy link
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Choose a reason for hiding this comment

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

LGTM - @muhammad-ali-e don't you need to update the tool's requirements.txt to use the intended SDK version?

tools/structure/src/config/properties.json Outdated Show resolved Hide resolved
tools/text_extractor/src/config/properties.json Outdated Show resolved Hide resolved
tools/classifier/src/config/properties.json Outdated Show resolved Hide resolved
@muhammad-ali-e
Copy link
Contributor Author

LGTM - @muhammad-ali-e don't you need to update the tool's requirements.txt to use the intended SDK version?

@chandrasekharan-zipstack It will be updated once we transition out of the beta phase

Copy link
Contributor

@gaya3-zipstack gaya3-zipstack left a comment

Choose a reason for hiding this comment

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

Changes look fine. Except for tool version changes suggested by @chandrasekharan-zipstack

Copy link

@muhammad-ali-e muhammad-ali-e merged commit c685aba into main Nov 22, 2024
3 checks passed
@muhammad-ali-e muhammad-ali-e deleted the Tool-SDK-FileStorage branch November 22, 2024 03:12
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.

3 participants