Skip to content

Conversation

@TheRaccoon123
Copy link

No description provided.

Copy link

@TimothyRou TimothyRou left a comment

Choose a reason for hiding this comment

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

fix #7

@beliaev-maksim
Copy link
Owner

@raccon123
please convert to "ready for review" when ready to review

@TimothyRou
Copy link

image

Copy link

@TimothyRou TimothyRou left a comment

Choose a reason for hiding this comment

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

Is this how?

@beliaev-maksim beliaev-maksim marked this pull request as ready for review January 12, 2022 07:57
README.md Outdated
### Get variable
Get value of variable _my_var_
Get value of variable _my_var_
Note Throws error KeyError when Enviroment variable is not found
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Note Throws error KeyError when Enviroment variable is not found
> Note: Throws error `KeyError` when Environment variable is not found
>

return ""
if not suppress_echo:
click.echo("Environment Variable '{}' does not exist".format(name))
raise KeyError
Copy link
Owner

Choose a reason for hiding this comment

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

I have some concerns how this will work in CLI implementation. It will be a long unreadable traceback.

I think we need to raise a KeyError for Python module and just return empty and error message in CLI

Copy link
Owner

Choose a reason for hiding this comment

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

also unittests fail with current implementation. Need to fix it as well

@TimothyRou
Copy link

No need to run unit test on This might work got unittest working on my machine.

@beliaev-maksim
Copy link
Owner

@raccon123 , @TimothyRou
please go through comments and do appropriate fixes

Comment on lines 46 to 47

#
Copy link
Owner

@beliaev-maksim beliaev-maksim Jan 14, 2022

Choose a reason for hiding this comment

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

Suggested change
#

def run_checks(self, user):
# check that variable does not exist before start
user_list = ["--user"] if user else []

Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change

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.

Have def get_variable(name, user): obey suppress_echo=True and throw an error like KeyError

3 participants