PythonkwargsS3minio開源TestingHumanitarian Tech

A Named Parameter Never Reaches **kwargs: Why Every Disaster Photo in HOT's Drone System Was Stored as the Wrong Type

· 5 min read
Table of Contents
  1. The function
  2. Python's binding rule
  3. The fix
  4. The test: bind the mock to the real signature
  5. What I did not do, also in the PR
  6. Rules to keep

drone-tm, from the Humanitarian OpenStreetMap Team (HOT), is a tasking manager for post-disaster aerial imagery: plan flights, collect what the drones shot, stitch it into a usable map. On September 2, 2026 the maintainer merged my PR #882. The bug: every uploaded photo was stored as application/octet-stream, so browsers downloaded it instead of displaying it.

The fix is one argument. What is interesting is why it was missing, because the argument was right there in the signature.

The function

def add_obj_to_bucket(
    bucket_name: str,
    file_obj: BytesIO,
    s3_path: str,
    content_type: str = "application/octet-stream",
    **kwargs,
):
    ...
    result = client.put_object(
        bucket_name, s3_path, file_obj, file_obj.getbuffer().nbytes, **kwargs
    )

The signature declares content_type. The docstring documents it. Callers pass it: arq/tasks.py sends application/zip for the QField export, projects/project_logic.py passes the browser's file.content_type for user uploads.

But the put_object call passes four positional arguments and **kwargs. content_type is not among them.

Python's binding rule

When you call add_obj_to_bucket(bucket, obj, path, content_type="image/jpeg", metadata={...}), Python does this in order:

  1. Positional arguments bind to bucket_name, file_obj, s3_path.
  2. For each keyword argument: if the signature has a parameter by that name, bind it there. Only if the signature does not have it does it go into **kwargs.

content_type is in the signature, so it binds to the local variable content_type. metadata is not, so it lands in kwargs. When the function calls put_object(..., **kwargs), kwargs holds only metadata. content_type sits quietly in a local that nobody forwards.

That is why metadata always worked and content_type never did. A named parameter is a gate: declaring it is a promise to forward it yourself. **kwargs only catches what the signature does not mention.

And the sibling function twelve lines up, add_file_to_bucket, already did it right: fput_object(..., content_type=content_type). Same file, two functions, one remembered and one forgot.

Maintainer spwoodcock asked exactly this in review: would kwargs not carry it? It would not. I put the rule above in the reply, and he approved.

The fix

    result = client.put_object(
        bucket_name,
        s3_path,
        file_obj,
        file_obj.getbuffer().nbytes,
        content_type=content_type,
        **kwargs,
    )

One argument. minio's SDK puts it on the object's Content-Type header, and browsers render the image.

The test: bind the mock to the real signature

This is the kind of bug that invites a test that passes for the wrong reason. The usual approach is to mock put_object and assert it was called, or assert content_type is in the call's kwargs. The first is always green. The second silently stops meaning anything the day minio changes its signature.

What I did instead was bind the call through the real SDK signature:

import inspect
from minio import Minio

class _RecordingClient:
    def __init__(self):
        self.calls = []

    def put_object(self, *args, **kwargs):
        bound = inspect.signature(Minio.put_object).bind(self, *args, **kwargs)
        bound.apply_defaults()
        self.calls.append(bound.arguments)
        return _Result()

inspect.signature(Minio.put_object).bind(...) resolves the call using minio's own signature: positionals, keywords and defaults all land where the SDK would put them. The assertion is on bound.arguments["content_type"], the value the SDK would actually send, not the value the wrapper believes it sent. If minio ever reorders or renames a parameter, bind raises TypeError and the test goes red instead of quietly passing.

A second test pins that metadata still arrives, because the QField caller passes both. Fixing content_type must not displace it.

Both tests fail before the change and pass after it. I ran the unmodified version first.

What I did not do, also in the PR

This machine cannot install the project's full backend. GDAL and Scrapy do not build here. So I ran the tests with --noconftest, which is enough because they touch nothing but app.s3 and minio. They need no database, Redis or docker and should run normally in CI.

If the maintainer preferred an integration test, the equivalent assertion is client.stat_object(bucket, key).content_type. I could not run that variant here, so I wrote in the PR that I would not claim it verified.

That paragraph is worth as much as the fix. A maintainer wants to know what you verified and what you did not, not a bare "tests pass".

Rules to keep

  1. A named parameter in the signature never reaches **kwargs. If you declared it, you forward it.
  2. A sibling function in the same file is the best control group. add_file_to_bucket had it right, add_obj_to_bucket did not, and the diff is the answer.
  3. Bind mocks to the real signature. inspect.signature(RealSDK.method).bind(...) lets you assert what the SDK actually receives, and it speaks up when the SDK changes.
  4. State the verification you did not run. "I would not claim it verified" tells the maintainer where to look, and gets merged faster than pretending.

The PR: hotosm/drone-tm#882, +70 / -1, of which 6 lines are production code and the rest tests. It is the second of seven PRs merged in 24 days.

Weekly AI Automation Playbook

No fluff — just templates, SOPs, and technical breakdowns you can use right away.

Join the Solo Lab Community

Free resource packs, daily build logs, and AI agents you can talk to. A community for solo devs who build with AI.

Need Technical Help?

Free consultation — reply within 24 hours.