Skip to content

fix: fix get_content() call for frappe v16 compatibility#44

Open
prateekkaramchandani wants to merge 1 commit into
developmentforpeople:developfrom
kaminds:develop
Open

fix: fix get_content() call for frappe v16 compatibility#44
prateekkaramchandani wants to merge 1 commit into
developmentforpeople:developfrom
kaminds:develop

Conversation

@prateekkaramchandani
Copy link
Copy Markdown

No description provided.

@niraj2477
Copy link
Copy Markdown

frappe does not check for read permission when-ever you do get_contents, but the overridden class does this.
This could break in production , we should remove downloadable check
@developmentforpeople

@niraj2477
Copy link
Copy Markdown

can we remove it in same pr?

Profit-Rocket added a commit to Floreer-Africa/dfp_external_storage that referenced this pull request May 13, 2026
Frappe v16 changed File.get_content() signature to accept an optional
encodings parameter (frappe/frappe/core/doctype/file/file.py:592). Our
DFPExternalStorageFile.get_content() subclass override didn't, so any
caller passing encodings=... raised TypeError on v16.

Forwards the parameter through to the parent for local-file lookups.
S3-backed downloads still return raw bytes (the encoding param doesn't
apply to remote object retrieval).

Mirrors upstream PR developmentforpeople#44, with
the typo corrected (upstream passes encodings=None which loses the
caller's request; we pass encodings=encodings so the parameter actually
propagates).

When upstream merges developmentforpeople#44 (or a successor), reconcile via merge —
this commit becomes a no-op or near-no-op.
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.

2 participants