Skip to content

Conversation

KazuCocoa
Copy link
Member

If the python environment has no twine, an exception will occur. The command only pushes the dist module to pypi. (Building the module is in another method)
It can run after other tasks, so let me show a message to push later when the command fails.

@KazuCocoa KazuCocoa requested a review from ki4070ma December 29, 2019 08:58
try:
call_bash_script('twine upload "{}"'.format(push_file))
except Exception as e:
print('Failed to upload {} to pypi. '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the error be printed to stderr?
should we rethrow the exception after print?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it was enough to print messages as stdout without stop/rethrow errors.
Current usage is showing messages on CLI and should not stop this by this error.

release.sh Outdated

CONFIGURE_DIR=$(dirname "$0")
"$PYTHON_BIN_PATH" "${CONFIGURE_DIR}/script/release.py" "$@"
"$PYTHON_BIN_PATH" -m "${CONFIGURE_DIR}/script/release.py" "$@"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my understanding, -m has no meaning.
Module name needs to follow after -m. (e.g. pip, pytest)

https://docs.python.org/3/using/cmdline.html

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. yeah, it's true

@ki4070ma
Copy link
Collaborator

@KazuCocoa

Just question

If the python environment has no twine,

In this case, doesn't this script exit during validate_release_env()?
Or no twine is different from not installed twine ?

@KazuCocoa
Copy link
Member Author

No, that was passed. I also expected the command stopped by it.
Yes. twine worked by python -m twine but did not only twine in release.py.
I assumed Python2.7/Python3/pyenv stuff... (Haven't dived deeply though)

@KazuCocoa KazuCocoa merged commit e16ab70 into appium:master Jan 4, 2020
@KazuCocoa KazuCocoa deleted the km/add-catch branch January 4, 2020 15:44
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