Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Add -s to pg:backups public-url so people feel better about scripting #1495

Merged
merged 2 commits into from
Apr 9, 2015

Conversation

chadbailey59
Copy link
Contributor

This adds an option to force hiding the "This link will expire in..." text from pg:backups public-url. This isn't entirely necessary, as we already check for TTY, but ruby can be tricky with that sometimes anyway. I'll be able to shut up more than a few support tickets with this.

@msakrejda
Copy link
Contributor

If we add this, we should probably call this -q / --quiet for consistency with UNIX tools (to the extent that there is any).

@will
Copy link
Contributor

will commented Apr 9, 2015

Could also move the text to stderr

@msakrejda
Copy link
Contributor

Incidentally, the linked TTY stuff is pretty straightforward (if not intuitive): a child process inherits stdin, stdout, and stderr from its parent. So it doesn't really matter where you're calling a process--only how you're redirecting output. So if you're going to be using the output somewhere that's not a TTY (e.g., redirecting it to another command, storing it in a variable, or using the output of a subshell invocation directly), it should just work correctly.

@chadbailey59
Copy link
Contributor Author

@uhoh-itsmaciek , I changed it to quiet. (I didn't like suppress anyway.)

@will re: our discussion in HipChat: I agree this isn't necessary because of the TTY stuff. But we've had public-url in the wild for a day now, and I already got a ticket about how "this extra line of text is going to break my scripts", and I know it won't be the first. Honestly, it was quicker for me to write this code than to figure out how to fit an explanation of the TTY behavior into heroku help pg:backups public-url.

That said, if consensus is that this is a bad idea, I'm happy to trash it.

@msakrejda
Copy link
Contributor

@chadbailey59 there are far worse command flags out there =)

I guess people may have incorrectly assumed that the extra quotes in the old command's output wouldn't break anything (when in fact things were not-breaking because of the special #tty? handling).

If it reduces support burden, I'm all for it. One could conceivably want terse output even if output is a TTY (say, to more easily copy and paste into something).

@@ -39,6 +39,7 @@ def copy
# capture DATABASE # capture a new backup
# restore [[BACKUP_ID] DATABASE] # restore a backup (default latest) to a database (default DATABASE_URL)
# public-url BACKUP_ID # get secret but publicly accessible URL for BACKUP_ID to download it
# -q, --quiet # Hide expiration message (for use in scripts)
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't appear to be in the right place

Copy link
Contributor

Choose a reason for hiding this comment

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

oops, nevermind, it is

jdx pushed a commit that referenced this pull request Apr 9, 2015
Add `-s` to `pg:backups public-url` so people feel better about scripting
@jdx jdx merged commit cb13738 into master Apr 9, 2015
@jdx jdx deleted the cb-suppress-pg-backups-message branch April 9, 2015 21:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants