Skip to content

fix Python updateprogress()#925

Closed
smlng wants to merge 2 commits intoOctopusDeploy:masterfrom
smlng:pr/python-updateprogress
Closed

fix Python updateprogress()#925
smlng wants to merge 2 commits intoOctopusDeploy:masterfrom
smlng:pr/python-updateprogress

Conversation

@smlng
Copy link
Copy Markdown

@smlng smlng commented Oct 7, 2022

According to the documentation the message for updateprogress in Python scripts is optional. However, when a message is not provided the script fails because the default for message is None which is then passed to the inner encode() function that does not like None.

In this fix the default value for message is changed from None to "" (empty string). Further this PR provided necessary test fixtures to assert the correct behavior.

smlng added 2 commits October 7, 2022 22:20
Documentation and code suggest that message is optional to updateprogress.
However, when not providing a message to updateprogress it fails with an
error from encode() as message is None. This is fixed here, by using an
empty string as default for message instead of None.
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 7, 2022

CLA assistant check
All committers have signed the CLA.

@smlng
Copy link
Copy Markdown
Author

smlng commented Oct 26, 2022

Ping, just some minor changes. Do I need to do something to move this forward.

@IsaacCalligeros95
Copy link
Copy Markdown
Collaborator

Thanks for the PR @smlng. I've created a fresh branch and PR, as our build scripts are exposed within this repository we do not automatically build community pull requests and have to trigger them manually. Some new security measures internally have prevented us from manually triggering these, and I'm waiting for confirmation whether I can. In the meantime, I've branched off of your changes and created a fresh pull request. I'll take a look at the changes early next week and see if we can get them merged in. Thanks again for the contribution!

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.

3 participants