Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add transaction rprompt #118

Merged
merged 6 commits into from Aug 30, 2019

Conversation

@guoweikuang
Copy link
Contributor

commented Aug 28, 2019

New redis transaction prompt, then the user will know he is in a transection.
#92

@@ -113,6 +123,12 @@ def gather_args(ctx, h, p, n, password, raw, cmd, decode):
When no command is given, redis-cli starts in interactive mode.
\b

This comment has been minimized.

Copy link
@laixintao

laixintao Aug 28, 2019

Owner

Excellent!

@laixintao
Copy link
Owner

left a comment

Excellent PR.

@@ -55,11 +57,13 @@ def repl(client, session, start_time):
_interval = None

try:
get_rprompt = lambda: '<transaction>' if in_transaction else None

This comment has been minimized.

Copy link
@laixintao

laixintao Aug 28, 2019

Owner

Do we really need a lambda here?

This comment has been minimized.

Copy link
@laixintao

laixintao Aug 28, 2019

Owner
Suggested change
get_rprompt = lambda: '<transaction>' if in_transaction else None
get_rprompt = '<transaction>' if in_transaction else None

This comment has been minimized.

Copy link
@laixintao

laixintao Aug 28, 2019

Owner

I think this line should be out of try-catch scope. It can't generate any Exceptions.

guoweikuang added 2 commits Aug 28, 2019
@laixintao

This comment has been minimized.

Copy link
Owner

commented Aug 28, 2019

There is a callback mechanism, means that if a command executed successfully, the callback will be executed to render output and do something else.

For example, update the completers, or update the config state.

key_completer.touch_words(str_items)

How about we use the global config to flag transaction/un-transaction, and use callback function to update the config?

@guoweikuang

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

This is a good idea, I will try

@laixintao

This comment has been minimized.

Copy link
Owner

commented Aug 28, 2019

We use pexpect to mock terminal test: https://pexpect.readthedocs.io/en/stable/

If it raise Timeout, it means iredis didn't write the output as we expected.

@guoweikuang

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

@laixintao May not be caused by the PR I submitted, I am looking for a reason

@laixintao

This comment has been minimized.

Copy link
Owner

commented Aug 30, 2019

Thanks! All looks good.

iredis/renders.py Outdated Show resolved Hide resolved
@laixintao

This comment has been minimized.

Copy link
Owner

commented Aug 30, 2019

There are some tricky situations here, like when user suffers a connection problem, and client raise an exception, I don't think we can set the transaction to False.

Merge for now, chekc later :)

@laixintao laixintao merged commit 6bc3624 into laixintao:master Aug 30, 2019

3 of 4 checks passed

ci/circleci: build-release CircleCI is running your tests
Details
ci/circleci: black-check Your tests passed on CircleCI!
Details
ci/circleci: flake8-check Your tests passed on CircleCI!
Details
ci/circleci: unittest Your tests passed on CircleCI!
Details
@laixintao laixintao referenced this pull request Aug 30, 2019
3 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.