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

core[patch]: rm image loading #27797

Merged
merged 5 commits into from
Oct 31, 2024
Merged

core[patch]: rm image loading #27797

merged 5 commits into from
Oct 31, 2024

Conversation

baskaryan
Copy link
Collaborator

@baskaryan baskaryan commented Oct 31, 2024

Patch for v0.3.x branch

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Oct 31, 2024
Copy link

vercel bot commented Oct 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Oct 31, 2024 5:27pm

@dosubot dosubot bot added Ɑ: core Related to langchain-core 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs labels Oct 31, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Oct 31, 2024
@baskaryan baskaryan requested a review from efriis October 31, 2024 17:31
@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Oct 31, 2024
@baskaryan baskaryan merged commit c1e7423 into master Oct 31, 2024
78 checks passed
@baskaryan baskaryan deleted the bagatur/rm_image_load branch October 31, 2024 17:34
return f"data:{mime_type};base64,{encoding}"
def __getattr__(name: str) -> Any:
if name in ("encode_image", "image_to_data_url"):
msg = f"'{name}' has been removed for security reasons."
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very cryptic!

  1. why did we remove encode_image / image_to_data_url are those not fine if users just use them as a utility? (i also get that they're really light weight so maybe we don't need them.)
  2. The error message is cryptic. Could we provide at least one more detail? It's not clear why the utility functions themselves would be a security concern.

The main concern is that this going to break user code, so would be ideal to provide information. Including the version as of which this change was made

@bachp
Copy link

bachp commented Nov 16, 2024

Is there any information why this was removed? As this breaks already working code in the 0.3 branch it would be nice to at least know why this is a security issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ɑ: core Related to langchain-core lgtm PR looks good. Use to confirm that a PR is ready for merging. 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants