Do not send empty fields in the upload form data

[dnicolodi]

No behavior changes intended. Just cleaning up some loose ends.

[sigmavirus24]

The underlying library strips none values if I remember correctly. I agree it's a strange behavior to rely upon but as the author, I have no intention of breaking that

[dnicolodi]

None values are encoded as empty strings in the form data, this is why some fields are already set only if they are not None. It is very confusing to have missing optional values indicated as None values in a dict. It is much more natural to do not have keys for items that are not set. Additionally, the receiving end of the form data needs to filter out set but empty form fields, for example https://github.com/pypi/warehouse/blob/9a270a2d01cba250795daf1e05133fbbafeeb7b0/warehouse/forklift/metadata.py#L293-L296

I have no intention of breaking that

What is that is this context?

[sigmavirus24]

What is that is this context?

https://toolbelt.readthedocs.io/en/latest/uploading-data.html#streaming-multipart-data-encoder

In other words: The thing that encodes the request data to PyPI which should be stripping out any None values thus meaning nothing is sent.

I agree that just not including the field is just as fine. But the different here is not in not sending it to PyPI, it's just in not including it in the dictionary.

[dnicolodi]

In other words: The thing that encodes the request data to PyPI which should be stripping out any None values thus meaning nothing is sent.

Sorry to insist, but this comment in the code made me believe otherwise:

twine/twine/package.py

Lines 274 to 278 in 0605ef0

	# FIPS disables MD5 and Blake2, making the digest values None. Some package
	# repositories don't allow null values, so this only sends non-null values.
	# See also: https://github.com/pypa/twine/issues/775
	if self.md5_digest:
	data["md5_digest"] = self.md5_digest

And testing suggests that fields with None value are not omitted from the encoded data:

>>> from requests_toolbelt.multipart.encoder import MultipartEncoder
>>> MultipartEncoder(fields=[('foo', None)]).to_string()
b'--f206243b92be45efa8c60a7c05060c28\r\nContent-Disposition: form-data; name="foo"\r\n\r\n\r\n--f206243b92be45efa8c60a7c05060c28--\r\n'

[sigmavirus24]

Hm, that's a bug in the Encoder but that will be hard to fix at this point, so this makes more sense in that case

[dnicolodi]

Thanks for the review and for merging, @sigmavirus24

Hm, that's a bug in the Encoder but that will be hard to fix at this point

I'm not entirely sure it is a bug. It has been ages since I had to deal with web-related stuff, but I have some vague recollection of empty fields in form data to not be an unusual occurrence, where the mere presence of the field in the data has a meaning. I don't know why someone would decide to model them as None values rather than empty strings, but I would not rule out that someone, somewhere, is doing precisely that.

[sigmavirus24]

Requests has always stripped None from anything except the json= parameter so I would consider this a bug but that's a matter of expectation setting