Version Upload API: Handle \n for release notes #262

Dalai Felinto · 2024-10-10T18:26:45+02:00

Dalai Felinto commented

2024-10-10 18:26:45 +02:00

On my tests, when running the API with CURL via command-line the \n
were being converted to string \n, and there would be no way to write a
multi-line release notes.

On my tests, when running the API with CURL via command-line the \n were being converted to string \n, and there would be no way to write a multi-line release notes.

Dalai Felinto added 1 commit 2024-10-10 18:26:46 +02:00

Version Upload API: Handle \n for release notes d8e48d9681

On my tests, when running the API with CURL via command-line the \n
were being converted to string \n, and there would be no way to write a
multi-line release notes.

Dalai Felinto commented

2024-10-10 18:30:18 +02:00

For the records, initially the unittest was passing even without my changes. It seems that CURL interprets strings differently than our unittest? It is also possible that I don't know how to test multi-lines with CURL (I tried a few ways, all of them failed, requiring this patch).

Dalai Felinto commented

2024-10-10 18:30:41 +02:00

And once (if) this is accepted I will update the documentation to mention \n: https://developer.blender.org/docs/features/extensions/ci_cd/

And once (if) this is accepted I will update the documentation to mention `\n`: https://developer.blender.org/docs/features/extensions/ci_cd/

Francesco Siddi requested changes 2024-10-10 18:40:41 +02:00

Dismissed

extensions/views/api.py Outdated

						
				@ -175,0 +175,4 @@

				    def validate_release_notes(self, value):

				        r"""Make sure \n and \r are valid after sanitation."""

				        if not value:

				            return value

Francesco Siddi commented

2024-10-10 18:39:54 +02:00

why not a simple return?

why not a simple `return`?

Dalai Felinto commented

2024-10-10 18:46:25 +02:00

To make sure if we get None we still return None, if we get "" we still return "".

I will change this so it test for None instead.

To make sure if we get None we still return None, if we get "" we still return "". I will change this so it test for None instead.

dfelinto marked this conversation as resolved

Dalai Felinto added 1 commit 2024-10-10 18:47:36 +02:00

From review: Simplify return 399f412e54

Francesco Siddi approved these changes 2024-10-10 18:51:18 +02:00

Dalai Felinto merged commit 3bdf37b6bd into main

2024-10-10 18:58:14 +02:00

Dalai Felinto deleted branch version-update-linebreak

2024-10-10 18:58:14 +02:00

Dalai Felinto referenced this issue from a commit

2024-10-10 18:58:15 +02:00

Version Upload API: Handle \n for release notes (#262)

Oleg-Komarov commented

2024-10-13 18:05:53 +02:00

@dfelinto do you have an example with the curl command you were using that was treated unexpectedly? I suspect that it may be a problem with shell escaping, i.e. that the request processing itself works correctly, but curl is being passed \ and n instead of \n.

I haven't run the code, but do I understand it correctly that now it is impossible to submit release notes that contain a literal \n sequence?
I can imagine someone publishing a release note about a fixed bug of "\n processing fixed" and not being able to do that because of this added change.

@dfelinto do you have an example with the curl command you were using that was treated unexpectedly? I suspect that it may be a problem with shell escaping, i.e. that the request processing itself works correctly, but curl is being passed `\` and `n` instead of `\n`. I haven't run the code, but do I understand it correctly that now it is impossible to submit release notes that contain a literal `\n` sequence? I can imagine someone publishing a release note about a fixed bug of "\n processing fixed" and not being able to do that because of this added change.

Dalai Felinto referenced this issue from a commit

2024-10-14 11:34:48 +02:00

Revert "Version Upload API: Handle \n for release notes (#262)"

Dalai Felinto commented

2024-10-14 11:40:47 +02:00

@Oleg-Komarov you are right. I reverted the commit and updated the documentation.

This works well now:

curl -X POST https://extensions.blender.org/api/v1/extensions/my_addon/versions/upload/ \
  -H "Authorization:bearer x_xRe91mBmlHLveA9v-xyKgPcm8Zig2EH7Uhh0vHCN8" \
  -F "version_file=@/home/user/my-addon-1.0.1.zip" \
  -F release_notes=$'* Fix crashes when opening Blender.\n* Improve performance 2x.\n* Fix parsing \\n characters.'

Thanks for keeping an eye on it.

@Oleg-Komarov you are right. I reverted the commit and updated the [documentation](https://developer.blender.org/docs/features/extensions/ci_cd/). This works well now: ``` curl -X POST https://extensions.blender.org/api/v1/extensions/my_addon/versions/upload/ \ -H "Authorization:bearer x_xRe91mBmlHLveA9v-xyKgPcm8Zig2EH7Uhh0vHCN8" \ -F "version_file=@/home/user/my-addon-1.0.1.zip" \ -F release_notes=$'* Fix crashes when opening Blender.\n* Improve performance 2x.\n* Fix parsing \\n characters.' ``` Thanks for keeping an eye on it.

Oleg-Komarov commented

2024-10-14 20:27:53 +02:00

Thank you!

Sign in to join this conversation.

No reviewers