Add quarterly business review demo#16
Conversation
events/next18/README.md
Outdated
| ## Getting started | ||
|
|
||
| * Follow the [Sheets API python quickstart](https://developers.google.com/sheets/api/quickstart/python) | ||
| * Make sure to save the `client-secrets.json` file in your working directory |
There was a problem hiding this comment.
We recently renamed this file to credentials.json. Either update your README and code or mention that they should rename it.
events/next18/README.md
Outdated
|
|
||
| // Add data from the stub customer service | ||
| $ python qbr_tool.py add_customers \ | ||
| --spreadsheet_id <your spreadsheet id> \ |
There was a problem hiding this comment.
Is the spreadsheet ID printed out as a result of the previous command? If so, mention that in the comments.
events/next18/README.md
Outdated
| developer project | ||
| * Run the tool with no arguments to complete the OAuth consent flow: | ||
|
|
||
| <pre> |
There was a problem hiding this comment.
Nitpick: More common to use tripple backticks instead of pre tags.
events/next18/README.md
Outdated
| * Run the tool with no arguments to complete the OAuth consent flow: | ||
|
|
||
| <pre> | ||
| $ python qbr_tool.py |
There was a problem hiding this comment.
Nitpick: Remove unnecessary leading whitespace, here and below.
events/next18/README.md
Outdated
| * Run the tool: | ||
|
|
||
| <pre> | ||
| // Create the spreadsheet from the Google Slides template. |
There was a problem hiding this comment.
Use a hash sign for comments, instead of double slash, to conform with bash syntax.
| def ExecuteRead(self): | ||
| filters = list(self._data_filters.values()) | ||
| get_body = {'dataFilters': filters} | ||
| read_fields = ('sheets.properties.sheetId,sheets.data.rowData.' + |
There was a problem hiding this comment.
Optional: Would it be more readable to define these as a list an then join them?
read_fields = ','.join([
'sheets.properties.sheetId',
'sheets.data.rowData.values.formattedValue',
'developerMetadata.metadataValue'
])| def GetColumnData(self, column_id): | ||
| index = list(self._data_filters.keys()).index(column_id) | ||
| data = self._spreadsheet.get('sheets')[0].get('data')[index] | ||
| values = [] |
There was a problem hiding this comment.
Optional: More compact, but perhaps less readable, to use a list comprehension:
values = [row.get('values')[0].get('formattedValue') for row in data.get('rowData')]
| self._requests.append(request) | ||
|
|
||
| def ExecuteBatchUpdate(self): | ||
| body = {'requests': self._requests} |
There was a problem hiding this comment.
Is there some number of requests where it would make sense to break them into multiple HTTP requests? AKA, how many placeholders could you have before this fails?
There was a problem hiding this comment.
I havent tested it in a while, but the intent is for this to be OK for ~thousands of placeholders.
|
added a commit that addresses the feedback, thanks for taking a look! |
events/next18/README.md
Outdated
| # Create the spreadsheet from the Google Slides template. | ||
| # For example, 13My9SxkotWssCc2F5yaXp2fzGrzoYV6maytr3qAT9GQ | ||
| $ python qbr_tool.py create_sheet --template_id <your template id>; | ||
| Spreadsheet URL: https://docs.google.com/spreadsheets/d/<spreadsheet id> |
There was a problem hiding this comment.
Nitpick: Put the output in a comment, to indicate they shouldn't paste it into the command line.
events/next18/qbr_tool.py
Outdated
| store = file.Storage('credentials.json') | ||
| creds = store.get() | ||
| if not creds or creds.invalid: | ||
| flow = client.flow_from_clientsecrets('client_secret.json', SCOPES) |
There was a problem hiding this comment.
Need to update this to credentials.json as well.
|
Note: I've fixed the Travis builds since the creation of this PR. |
|
Travis is now green, ptal! |
3 participants
No description provided.