Skip to content

DFP External Storage — get_content() encodings compatibility #45

@Priyanshu-Arora-AI

Description

@Priyanshu-Arora-AI

Environment

  • App: dfp_external_storage (File doctype override)
  • Affected: Any code that calls File.get_content(encodings=...) when the File is overridden by DFPExternalStorageFile
  • Example caller: india_compliance (GSTR-1 generation and GST Return Log file download)

Problem

Frappe core File defines:

def get_content(self, encodings=None) -> bytes | str:

Callers (e.g. India Compliance) use file.get_content(encodings=[]) to get raw bytes (no decoding). This is required for gzip-compressed JSON and binary content.

DFPExternalStorageFile overrides get_content() without the encodings parameter:

def get_content(self) -> bytes:

So when the file is served by DFP External Storage (e.g. S3), the call fails with:

TypeError: DFPExternalStorageFile.get_content() got an unexpected keyword argument 'encodings'

Steps to reproduce

  1. Install india_compliance and dfp_external_storage; ensure File is overridden by DFPExternalStorageFile (hook).
  2. Use a GST Return Log (or any flow that stores a file and later calls get_file_doc(...).get_content(encodings=[])).
  3. Trigger GSTR-1 generation (or download a stored file that goes through DFP External Storage).
  4. Observe TypeError in traceback.

Expected behavior

DFPExternalStorageFile.get_content() should accept the same signature as Frappe’s File.get_content(encodings=None) and pass encodings through when delegating to the parent, so all callers that use encodings continue to work.

Impact

  • India Compliance: GSTR-1 generation and GST Return Log file download fail when files are stored via DFP External Storage.
  • Any other app that calls file.get_content(encodings=...) will break when the file is a DFPExternalStorageFile.

---

## Resolution method (for maintainers / PR description)


### Fix
Update `DFPExternalStorageFile.get_content()` to accept and forward the `encodings` parameter so it remains API-compatible with `File.get_content()`.

**File:** `dfp_external_storage/dfp_external_storage/doctype/dfp_external_storage/dfp_external_storage.py`

**Change:**

1. **Method signature:** Add `encodings=None`:
   ```python
   def get_content(self, encodings=None) -> bytes:
  1. Docstring: Note that encodings is optional, same as File.get_content; pass encodings=[] for raw bytes; for S3 files it is ignored (content is always bytes).

  2. Local files: When delegating to the parent, pass encodings through:

    if not self.dfp_is_s3_remote_file():
        return super(DFPExternalStorageFile, self).get_content(encodings=encodings)
  3. S3 path: No change; continue to return bytes from dfp_external_storage_download_file(). The encodings argument can be ignored for S3 (binary content).

Result: Callers like India Compliance that use file.get_content(encodings=[]) work for both standard File and DFPExternalStorageFile (local and S3).


---

## Quick copy-paste summary

| Field        | Content |
|-------------|---------|
| **Repo**    | dfp_external_storage (or wherever DFPExternalStorageFile is maintained) |
| **Title**   | `DFPExternalStorageFile.get_content() does not accept encodings parameter — breaks compatibility with Frappe File API and callers (e.g. India Compliance)` |
| **Labels**  | bug, compatibility, file |
| **Body**    | Use the “Description” section above. |
| **Fix**     | Use the “Resolution method” section in a PR or for maintainers. |

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions