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

Dialogflow API, support for versions and environments #2622

Open
lorenzosfarra opened this issue Jan 21, 2020 · 7 comments
Open

Dialogflow API, support for versions and environments #2622

lorenzosfarra opened this issue Jan 21, 2020 · 7 comments

Comments

@lorenzosfarra
Copy link

@lorenzosfarra lorenzosfarra commented Jan 21, 2020

Hi,

I would like to integrate the Dialogflow environments feature in my code.
The feature is currently in beta, but I think it should be possible somehow to try it (sorry if I have missed how).

Currently, when getting a session, we use the following code (kind of):

$sessionClient = new SessionsClient();
$session = $sessionClient->sessionName($projectId, $sessionId);

Where the method sessionName() is defined in src/V2/Gapic/SessionsGapicClient.php as:

public static function sessionName($project, $session)
    {
        return self::getSessionNameTemplate()->render([
            'project' => $project,
            'session' => $session,
        ]);
    }

and getSessionNameTemplate() as:

private static function getSessionNameTemplate()
    {
        if (null == self::$sessionNameTemplate) {
            self::$sessionNameTemplate = new PathTemplate('projects/{project}/agent/sessions/{session}');
        }

        return self::$sessionNameTemplate;
    } 

Now, per documentation, we should:

alter the endpoint URL by inserting environments/environment-name/users/-/ between agent and sessions.

A possible solution is something like this (not tested, just guessing logically):

First, should be modify the sessionName method as follows to add a possible 3rd param, the environment, and use a different template in case it's not null:

public static function sessionName($project, $session, $environment = null)
    {
        if ($environment) {
            return self::getSessionNameWithEnvironmentTemplate()->render([
                'project' => $project,
                'environment' => $environment,
                'session' => $session,
            ]);
        }
        return self::getSessionNameTemplate()->render([
            'project' => $project,
            'session' => $session,
        ]);
    }

and to create a getSessionNameWithEnvironmentTemplate() method as follows:

private static function getSessionNameWithEnvironmentTemplate()
    {
        if (null == self::$sessionNameTemplate) {
            self::$sessionNameTemplate = new PathTemplate('projects/{project}/agent/environments/{environment}/users/-/sessions/{session}');
        }

        return self::$sessionNameTemplate;
    } 

Let me know if beta features are not available in this SDK by choice, if I have missed it somewhere or if simply this issue makes sense :)

Thanks,
Lorenzo

@lorenzosfarra

This comment has been minimized.

Copy link
Author

@lorenzosfarra lorenzosfarra commented Jan 21, 2020

The code above is good enough to generate the new endpoint URL but it's not enough.
The RequestBuilder fails because

Could not map bindings for google.cloud.dialogflow.v2.Sessions/DetectIntent to any Uri template

Now, the uriTemplate is specified in src/V2/resources/sessions_rest_client_config.php, and it should follow RFC6570.
I don't see right now a way to specify an optional path segment (environments/{environment}/users/- should be the optional part).

If I force the uriTemplate to 'uriTemplate' => '/v2/{session=projects/*/agent/environments/*/users/-/sessions/*}:detectIntent', the code continues, but eventually I get a 404 error from the webhook stating that:

The requested URL \/v2\/projects\/\/agent\/environments\/\/users\/-\/sessions\/:detectIntent<\/code> was not found on this server.

While I am waiting (and hoping) for a positive response, I continue to investigate further :)

@jdpedrie

This comment has been minimized.

Copy link
Member

@jdpedrie jdpedrie commented Jan 21, 2020

Hi @lorenzosfarra,

The REST URI bindings do not appear to have been updated in the API definition for this feature. However, I believe that it should work if you use gRPC. Have you tried installing the gRPC extension?

You could bypass the formatting method (getSessionName()) entirely and build the session name yourself.

I've not been able to dig into this deeply yet, but I wanted to reply and let you know we see it and hopefully help get you moving. If my suggestion doesn't work let me know; I'll keep working on it as soon as I'm able.

@lorenzosfarra

This comment has been minimized.

Copy link
Author

@lorenzosfarra lorenzosfarra commented Jan 22, 2020

Hi and thanks for your reply and support!

I didn't thought about this way to do it (not used gRPC directly).
I will study it and I will come back to you to let you know if it works. In the meantime, if you'll have the chance to look more deeply regarding this subject, I will be more than happy :)

Thanks again.

@jdpedrie

This comment has been minimized.

Copy link
Member

@jdpedrie jdpedrie commented Jan 24, 2020

@lorenzosfarra were you able to proceed using gRPC?

@lorenzosfarra

This comment has been minimized.

Copy link
Author

@lorenzosfarra lorenzosfarra commented Jan 25, 2020

@lorenzosfarra

This comment has been minimized.

Copy link
Author

@lorenzosfarra lorenzosfarra commented Jan 27, 2020

Hi John,

I did some tests, and it seems they are successful, you were totally right.
I wanted to reply as soon as possible to update you about this issue and to have some feedbacks from you, if any.
I will perform more tests today and during the next days: I still don't feel totally confident to mark the issue as solved, even if it seems so...I will do it in a couple of days if you are OK with it, after some more tests on every aspect of the Dialogflow communication we have.

Let me know if I need to open a feature request or something in order to have this environment-ready process integrated somehow in this library. EDIT: https://issuetracker.google.com/issues/125939238 seems to be the same feature request!

Thank you very much for your support!


I don't know if it's the right place to share this information, but maybe it can be of some help for other persons having the same problem.

Based on the Install gRPC for PHP doc, on our server (running Ubuntu server 19.10, PHP7.3) I had to install some packages:

sudo apt-get install autoconf libz-dev php-dev php-pear
sudo pecl install grpc
sudo pecl install protobuf

and modify /etc/php/7.3/fpm/php.ini adding:

extension=grpc.so
extension=protobuf.so

Restarting php7.3-fpm:

sudo systemctl restart php7.3-fpm.service

Ad adding the "grpc/grpc": "^v1.1.0" line to the project composer.json.

As you said, I have been able to change the way I generate session names from:

$sessionClient = new SessionsClient();
$session = $sessionClient->sessionName($projectId, $sessionId);
$response = $sessionsClient->detectIntent($session, $queryInput);

to

$session = $this->_getSessionName($sessionId);
$sessionsClient = new SessionsClient();
$response = $sessionsClient->detectIntent($session, $queryInput);

// WHERE _getSessionName() is:
private function _getSessionName($sessionId)
{
    $projectId = $this->_getProjectId();
    $environment = $this->_getEnvironment();
    return "projects/{$projectId}/agent/environments/{$environment}/users/-/sessions/{$sessionId}";
}

It seems to work just fine.

@jdpedrie

This comment has been minimized.

Copy link
Member

@jdpedrie jdpedrie commented Jan 27, 2020

I'm so glad that worked for you! Sometimes brand new features take some time to bring full support to the REST transport. When you're working with cutting-edge stuff, it's generally best to prefer gRPC if at all possible.

I'd suggest dropping a comment in that issue you linked and let them know that while the clients now offer some support for versions and environments, the REST bindings need to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.