-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 appengine/bigquery sample #2
Conversation
# Copyright 2015 Google Inc. All rights reserved. | ||
# Use of this source code is governed by the Apache 2.0 | ||
# license that can be found in the LICENSE file. | ||
application: gae-bq-sample |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still required for App Engine? I think you can specify it on the command line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
// datasets returns a list with the ids of all the Big Query datasets visible | ||
// with the given context. | ||
func datasets(c context.Context) ([]string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c
--> ctx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Change-Id: If75ceaea96e3c6d58d1685a64117e4e43294d531
applied @broady's feedback
} | ||
|
||
// create a new App Engine context from the request. | ||
c := appengine.NewContext(r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want c
-> ctx
.
(ctx
because we're using the actual context
package now.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Change-Id: If8f8b10f4e0d9aae1e604fb06e3e8ad65802e76a
More feedback
merged e317f76 |
FYI @broady @markmandel