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

[Storage] Fix storage table outputs and help text. #1402

Merged
merged 4 commits into from
Nov 22, 2016
Merged

[Storage] Fix storage table outputs and help text. #1402

merged 4 commits into from
Nov 22, 2016

Conversation

tjprescott
Copy link
Member

Fixes #1066, fixes #1003, fixes #1002, fixes #1266.

Creates custom callables for various table output formats in storage. Updates help text.

@tjprescott tjprescott added the Storage az storage label Nov 21, 2016
@mention-bot
Copy link

@tjprescott, thanks for your PR! By analyzing the history of the files in this pull request, we identified @JasonRShaver, @derekbekoe and @BurtBiel to be potential reviewers.

Copy link
Member

@derekbekoe derekbekoe left a comment

Choose a reason for hiding this comment

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

Added a question but overall looks good

new_entry['Name'] = item_name
new_entry['Content Length'] = ' ' if is_dir else item['properties']['contentLength']
new_entry['Type'] = 'dir' if is_dir else 'file'
new_entry['Last Modified'] = item['properties']['lastModified'] or ' '
Copy link
Member

Choose a reason for hiding this comment

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

Will item['properties']['lastModified'] always exist?
Or is this check to see if item['properties']['lastModified'] is False, empty etc.?

Otherwise, you may get a KeyError.

Copy link
Member Author

@tjprescott tjprescott Nov 22, 2016

Choose a reason for hiding this comment

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

If lastModified is null, then what happens is the table just leaves that column off. The or ' ' ensures that it at least has a space so you will get a blank "cell". Otherwise your table dynamic when it really isn't.

Copy link
Member

Choose a reason for hiding this comment

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

Okay that works

@tjprescott tjprescott merged commit a1130a8 into Azure:master Nov 22, 2016
@tjprescott tjprescott deleted the StorageTableFormats branch November 22, 2016 17:52
thegalah pushed a commit to thegalah/azure-cli that referenced this pull request Nov 28, 2016
* Azure/master: (39 commits)
  User should use no-cache so we build a fresh image (Azure#1455)
  Bump all modules to version 0.1.0b10 (Azure#1454)
  [Docs] Move around the order of install instructions. (Azure#1439)
  acs: *_install_cli mark cli as executable (Azure#1449)
  Fix resource list table. (Azure#1452)
  [Compute] VM NIC updates (Azure#1421)
  Introduce batch upload and download for blob (Azure#1428)
  Add auto-registration for resource providers. (Azure#1330)
  interpret the '@' syntax if something higher hasn't already done that. (Azure#1423)
  Aliasing plan argument with shorthand (Azure#1433)
  ad:fix one more place which still uses localtime for secret starttime (Azure#1429)
  Add table formatting for deployments and sort by timestmap. (Azure#1427)
  Add table formatting for resource group list (and add 'status') (Azure#1425)
  [Docs] Add shields specifying latest version and supported python versions (Azure#1420)
  Add new az storage blob copy start-batch command (Azure#1250)
  Component Discovery (Azure#1409)
  Add poison logic to prevent re-recording tests that need updating. (Azure#1412)
  [Storage] Fix storage table outputs and help text. (Azure#1402)
  [mention-bot] Attempt to fix config (Azure#1410)
  ad:use utc time on setting app's creds (Azure#1408)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants