Skip to content
This repository has been archived by the owner on Oct 19, 2023. It is now read-only.

Add Flags That Enable Better Compatibility With Scripts #13

Merged
merged 1 commit into from
May 8, 2017
Merged

Add Flags That Enable Better Compatibility With Scripts #13

merged 1 commit into from
May 8, 2017

Conversation

ryan-clancy
Copy link
Contributor

@ryan-clancy ryan-clancy commented May 5, 2017

This PR contains some of the changes I made in my fork that I find to be quite useful.

This change adds some new flags that enable better compatibility with running the assistant in a script:

  • --interactive/--non-interactive - this allows setting the need for pressing enter between each response (defaults to true, behaves as it currently does)
  • --once - this makes it so that only one conversation occurs per run of the assistant (defaults to false, behaves as it does currently)

Normally, --non-interactive and --once would be passed together such that a single non-interactive conversation would take place (i.e. in the use-case of binding the assistant to run when a key is pressed).

Thoughts?

@proppy
Copy link
Contributor

proppy commented May 5, 2017

I wonder if we need two different flags? It sounds confusing for the sample to listen for a new query if continue_conversation == False.

Would adding --once and defaulting to wait_for_user_trigger = False if it set fulfill your main use case?

@ryan-clancy
Copy link
Contributor Author

You're right, changing the default of wait_for_user_trigger and just adding the --once flag would satifsy this use case.

Would you like me to remove the --interactive/--non-interactive flag commit? This would mean that, even though the first conversation wouldn't need a user trigger, each subsequent one (when not using --once), would need to wait for the trigger.

@proppy
Copy link
Contributor

proppy commented May 5, 2017

This would mean that, even though the first conversation wouldn't need a user trigger, each subsequent one (when not using --once), would need to wait for the trigger.

I believe the first conversation always need a trigger with the current code.

@ryan-clancy
Copy link
Contributor Author

I misread your previous message and thought you meant change the value of wait_for_user_trigger to False even if not set, my mistake.

Take a look at the latest commit, it has only the once flag and sets wait_for_user_trigger to False (not once) if the flag is passed (where the flag defaults to False).

@@ -214,11 +214,13 @@ def gen_converse_requests(self):
@click.option('--grpc-channel-option', multiple=True, nargs=2,
metavar='<option> <value>',
help='Options used to construct gRPC channel')
@click.option('--once', default=False, is_flag=True,
help='Force termination after a single response.')
Copy link
Contributor

Choose a reason for hiding this comment

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

s/response/conversation/

Copy link
Contributor

@proppy proppy left a comment

Choose a reason for hiding this comment

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

just a nit.

Additionally, don't wait for a user trigger if the 'once' flag is passed.
@ryan-clancy
Copy link
Contributor Author

Fixed.

@kadeve
Copy link
Contributor

kadeve commented May 6, 2017

I dont recommend using it without trigger because I ended up using my daily tokens within 1 hour

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants