Add gcs example to language #177
Conversation
| <info>php %command.full_name% Text to analyze.</info> | ||
| <info>php %command.full_name% gs://my_bucket/file_with_text.txt</info> |
There was a problem hiding this comment.
underscores are invalid in GCS buckets!!
| $text = implode(" ", $input->getArgument('text')); | ||
| $result = analyze_entities($text); | ||
| $content = implode(' ', (array) $input->getArgument('content')); | ||
| if (preg_match('/gs:\/\/([a-z|0-9|\.|-]+)\/(\S+)/', $content, $matches)) { |
There was a problem hiding this comment.
Add a comment explaining the regexp.
There was a problem hiding this comment.
Repeat for similar code below.
|
|
||
| /** | ||
| * Command line utility to transcribe. | ||
| * Command line utility for the Natural Language APIs. |
There was a problem hiding this comment.
Small nit: Is this really a utility? I'd call it command line sample or somthing
There was a problem hiding this comment.
It is both a utility and a sample. I do not think we need to change this.
| * Command line utility for the Natural Language APIs. | ||
| * | ||
| * Usage: php speech.php transcribe | ||
| * Usage: php language.php entities |
There was a problem hiding this comment.
This usage is not complete because there is argument. Remove it, or make it complete.
| 'content', | ||
| InputArgument::IS_ARRAY | InputArgument::REQUIRED, | ||
| 'Text to analyze' | ||
| 'Text or path to Cloud Storage file' |
There was a problem hiding this comment.
I feel little bit weird to use the same argument for the text and the gs path.
I think it's simpler if you make the text argument OPTIONAL (actually it's conditionally required), and add an option --gs-path or something?
If the text contains the sub-text "gs://your-bucket/your-object", does it match the regex?
For example, can you analyze the following text?
To analyze the text stored in cloud storage, specify the gs path like gs://your-bucket/your-object as the argument.
There was a problem hiding this comment.
Good catch on the regex! This is now fixed.
As for the --gcs-path, I experimented with this but I found it to be cumbersome. I think the current approach works better.
| <info>php %command.full_name% Text to analyze.</info> | ||
| <info>php %command.full_name% gs://my-bucket/file_with_text.txt</info> |
There was a problem hiding this comment.
The sub-command everything sounds a bit weird.
There was a problem hiding this comment.
What do you suggest? analyze-all?
| if (!self::$hasCredentials) { | ||
| $this->markTestSkipped('No application credentials were found.'); | ||
| } | ||
| if (!$gcsFile = getenv('GOOGLE_LANGUAGE_GCS_FILE')) { |
There was a problem hiding this comment.
Did you add the envvar on travis and jenkins?
bshaffer
force-pushed
the
add-gcs-example-to-language
branch
from
0dac7f9 to
696ac34
Compare
- Renames command classes for consistency
bshaffer
force-pushed
the
add-gcs-example-to-language
branch
from
696ac34 to
d328a8d
Compare
|
@tmatsuo PTAL |
3 participants
Updates CLI to accept
gs://path to language file