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 Python Hello World sample. [DO NOT MERGE] #173

Closed
wants to merge 4 commits into from
Closed

Conversation

tswast
Copy link
Contributor

@tswast tswast commented May 25, 2016

Uses the HappyBase layer, as recommended by Bigtable eng.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 25, 2016
This runs the hello.py script in a fresh Python "virtual environment"
and checks that the output matches what we expect.
@tswast
Copy link
Contributor Author

tswast commented May 25, 2016

@lesv PTAL.

@@ -0,0 +1,67 @@
# Cloud Bigtable Hello World

This is a simple application that demonstrates using the [Google Cloud Client
Copy link
Contributor

Choose a reason for hiding this comment

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

gcloud sdk?

@lesv
Copy link
Contributor

lesv commented May 26, 2016

We may decide (tomorrow's chat) to put this in python-docs-samples. I'm basically happy. Surprised how simple this is.

@jonparrott PTAL

Delete table Hello-Bigtable-1234
```

## Understanding the code

Choose a reason for hiding this comment

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

Doesn't this stuff usually go in the docs? Do you think it's worthwhile to duplicate this here or would it be better to link to the documentation page that describes this sample?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to see this stuff in the README as well as the docs. Code should always have everything you need, IMHO.

Choose a reason for hiding this comment

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

I think "everything you need" is satisfied by a link to the tutorial for this sample, yeah? Otherwise we risk the "official" tutorial in our docs and this version of the tutorial drifting.

If this does not yet have a companion documentation tutorial, then this can remain.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea - I prefer duplication as sometimes different ways of saying the same thing makes it easier to get. (And when I'm working, I prefer to look at my IDE and not a web page.

Choose a reason for hiding this comment

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

Fair enough, I'm okay with it as long as it stays relatively minimal.

@lesv
Copy link
Contributor

lesv commented May 27, 2016

LGTM - You need to take a look, I'm going to update master on your branch, however.

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels May 27, 2016
@lesv
Copy link
Contributor

lesv commented May 27, 2016

It looks like there is still a bit of work to do. I've updated your branch so it's possible to merge, but we should still get the license and Jon's changes.

@tswast
Copy link
Contributor Author

tswast commented May 27, 2016

Agreed. Please don't merge yet. I'll address Jon's comments. It sounds like I can get rid of the script and just use py.test.

@tswast tswast changed the title Add Python Hello World sample. Add Python Hello World sample. [DO NOT MERGE] May 27, 2016
@tswast
Copy link
Contributor Author

tswast commented Jun 6, 2016

I'm closing this PR. I'll send one soon for python-docs-samples.

@tswast tswast closed this Jun 6, 2016
@tswast tswast deleted the swast-python branch June 6, 2016 20:58
garye pushed a commit to garye/cloud-bigtable-examples that referenced this pull request Jul 28, 2016
* Initial empty repository
* Initialize the repository.

Adds shared pom.xml and google-checks.xml. I also add a test directory
to check that the pom works / is well-formed.

* Add Travis configuration.
* Remove unnecessary properties.
* Ignore generated AutoValue classes from checkstyle.
* Ignore JavaDoc rules in test classes.
* Ignore service account file.
* Add license header to POM
* Add instructions for using with subtrees.
Subtrees are the primary way I expect people to use the
`java-repo-tools` repository. Since it's not a super common workflow, I
document the needed steps in the README.
* Remove extra newline in README
* Remove another extra newline.
* Checkstyle fixes.

I modify the checkstyle config to have SAME_PACKAGE imports appear
before third-party imports. I think this better matches the intent of
the import ordering rules in the style guide.
https://google.github.io/styleguide/javaguide.html#s3.3.3-import-ordering-and-spacing
Expecially since it says the `com.google` imports only appear after
static imports if the source file is in the `com.google` package space.

(cherry picked from commit 7ca0e7564d4d1a39714d8c90e71d5e408f30f4da)

* Remove command that was copy-paste error.
The pulling changes from Java Repository Tools instructions can now be followed verbatim.
* Update README.md
* Add much easier steps for pulling in changes
The instructions here are much easier to follow and avoid unnecessary conflicts: https://help.github.com/articles/about-git-subtree-merges/
* Remove recommendation to create a branch.
* Added few more sample code for Search API. (GoogleCloudPlatform#173)
* Moved some checkstyle modules to java-repo-tools
* Moved some lines in .gitignore to the top level
* Add sample for App Engine  Datastore setDistinct projection queries.

Also update checkstyle to allow 1-letter variable names, since it
doesn't make sense to give long names to something like "arbitrary
column A that doesn't actually mean anything".

* Update coveralls plugin for code coverage analysis. (GoogleCloudPlatform#194)
* Use codecov for coverage. (GoogleCloudPlatform#199)
* Add Datastore indexes samples. (GoogleCloudPlatform#214)

Samples are moved from:
https://cloud.google.com/appengine/docs/java/datastore/indexes

Note: I add a new script `test-devserver.sh` to the testing config. This
script runs the `mvn appengine:devserver` plugin, waits for it to start,
then verifies that it gets a non-error response from the `/` path. I use
this to verify that the `datastore-indexes.xml` files are correct (by
disabling autoGenerate, the local devserver throws an error when a query
is used without the correct index defined, just as production does).

We should probably add any "hello world" or other projects to this
check, as well. Anywhere we say to run `mvn appengine:devserver`, it
would be good to test that it is correct.

* Use the subtree command to push changes upstream.
* Fix checkstyle errors in java/dataflow-coinbase sample.

This is an example of using the java-repo-tools parent pom in a sample.

To fix indentation and other simple errors, I ran the google-java-format
tool. https://github.com/google/google-java-format I still had to fix
imports, variable name, and javadoc checkstyle errors by hand.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: no This human has *not* signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants