Skip to content

Conversation

@jmdobry
Copy link
Member

@jmdobry jmdobry commented Sep 28, 2016

These are intended to be simple examples that explicitly show:

  • Importing the client library
  • Instantiating a client
  • Making a single API call
  • Printing the result

Products covered:

  • BigQuery
    • - sample
    • - test
  • Datastore
    • - sample
    • - test
  • Language
    • - sample
    • - test
  • Logging
    • - sample
    • - test
  • Pub/Sub
    • - sample
    • - test
  • Storage
    • - sample
    • - test
  • Translate
    • - sample
    • - test
  • Vision
    • - sample
    • - test

For reference, see the corresponding samples in other languages:

Thanks!

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 28, 2016
@theacodes
Copy link
Contributor

It's sad to see that there are no tests for these, especially considering these will be the first samples users try out.

@jmdobry
Copy link
Member Author

jmdobry commented Sep 28, 2016

It's sad to see that there are no tests for these, especially considering these will be the first samples users try out.

I'm willing to write tests, but I need a crash course on testing python-docs-samples.

@jmdobry jmdobry self-assigned this Sep 28, 2016
@theacodes
Copy link
Contributor

I'm willing to write tests, but I need a how to test python-docs-samples crash course.

I can add them, they're just inherently hard to test. As soon as you import these modules they do stuff (side-effects), this is why all of samples are in functions - so that tests can setup fixtures to mock whatever is needed or setup environment variables before calling the function.

You've also hard-coded the project id, which means I have to do at least some mocking. It's also inconsistent with all of the parameters shown in other samples (which are just function args).

@jmdobry
Copy link
Member Author

jmdobry commented Sep 28, 2016

I recognize that they're harder to test (the tests for Node.js versions of these samples were a pain). Perhaps @omaray can concede some simplicity for more maintainable/testable samples? i.e. wrap in a function?

Hard-coding the project ID is a "product requirement" for these samples (these "quickstart" style samples will be the only ones that do so, all the rest can infer from the environment).

Also, I tried TESTING.md, but sudo pip install --upgrade nox-automation fails on my machine, and gcp-python-repo-tools gives me a 404.

@jmdobry jmdobry changed the title Add some simple "quickstart" samples. [DO NOT MERGE] Add "quickstart" samples Sep 29, 2016
@omaray
Copy link

omaray commented Sep 29, 2016

@jonparrott happy to chat in person about this.

@theacodes
Copy link
Contributor

theacodes commented Sep 29, 2016

@omaray @jmdobry I think it's fine, it's just very hard to test cleanly. Of course, the usability of our samples is outweighs test cleanliness.

I have no time to write tests for this PR this week, what is the timeline you guys want for this to be merged?

@omaray
Copy link

omaray commented Sep 29, 2016

We were hoping this week if possible. But we can also check-in something for now that works (that we test manually to confirm) and then add the test next week. That wouldn't be a concern to me.

By the way, @jmdobry is using the "gcloud" module in his examples. Can we now switch to using the google-cloud-{api} one instead? So from google.cloud import pubsub and what not?

@jmdobry
Copy link
Member Author

jmdobry commented Sep 30, 2016

I can add them, they're just inherently hard to test. As soon as you import these modules they do stuff (side-effects), this is why all of samples are in functions - so that tests can setup fixtures to mock whatever is needed or setup environment variables before calling the function.

With python, can the test do the setup stuff before import the sample? (That's essentially what we do in Node.js.) Are imports required to be the first statements in a file?

@theacodes
Copy link
Contributor

With python, can the test do the setup stuff before import the sample? (That's essentially what we do in Node.js.) Are imports required to be the first statements in a file?

We can do that, it's just pretty hacky.

We were hoping this week if possible. But we can also check-in something for now that works (that we test manually to confirm) and then add the test next week. That wouldn't be a concern to me.

That would be fine.

@theacodes
Copy link
Contributor

@jmdobry if you rebase on master, you can use google-cloud (and tests should pass)

@jmdobry
Copy link
Member Author

jmdobry commented Sep 30, 2016

Done

@jmdobry
Copy link
Member Author

jmdobry commented Sep 30, 2016

I removed the explicit project IDs.

@jmdobry
Copy link
Member Author

jmdobry commented Oct 5, 2016

I wrapped each snippet in a method definition, does that make it easier for you to test? I also added snippets for Language and Vision. Speech is the only one missing (google-cloud-speech not published yet, but the docs for it have been published?)

@jmdobry
Copy link
Member Author

jmdobry commented Oct 5, 2016

Also, I ran all of the samples manually to make sure they work. Except for Speech, this PR now has "quickstart" sample parity with GoogleCloudPlatform/ruby-docs-samples#77 and the corresponding Node.js samples (which are already on master of nodejs-docs-samples).

@jmdobry jmdobry changed the title [DO NOT MERGE] Add "quickstart" samples Add new "quickstart" samples Oct 5, 2016
Change-Id: I5f1b0ff6396226aab54b0363d05e50cc3f49dea7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants