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

fix(pg-v5): return child process stdout #1701

Merged
merged 1 commit into from
Dec 16, 2020

Commits on Dec 16, 2020

  1. fix(pg-v5): return child process stdout

    https://heroku.support/945120
    
    In [PR #1691][1], some refactoring was done to make the way psql is
    executed more consistent. Unfortunately, this ended up in a regression
    for commands that were expecting the result of `PSQL.exec` to return a
    promise that resolved with the result of the psql child_process's stdout
    stream. While the output is displayed, some commands, [like `ps` were
    looking at the return result of `psql.exec`][2]. Because #1691 made this
    return `undefined`, downstream commands like `ps` or others now error.
    
    This changes the behavior in `packages/pg-v5/lib/psql.js` back to  what
    it was doing before:
    
    * Resolve the promise returned from `psql.exec` with the value of the
    psql child_process's `stdout` stream
    * lib/psql.js no longer pipes to stdout. This is once again the
    responsibility of the caller.
    
    Ideally, long term, we would return the child process (or a wrapper) and
    let callers decide to do with output themselves. Node has a lot of great
    utilities to make this easier. I tried going down this route but it
    ended up being a larger change than intended due to tests. In the
    interest of fixing this issue quickly, let's go with this approach until
    we can revisit the return value of `PSQL.exec` in the future.
    
    [1]: #1691
    [2]: https://github.com/heroku/cli/blob/729acd6382b424649c51bff7f4afb02df99d46ac/packages/pg-v5/commands/ps.js#L27
    fivetanley committed Dec 16, 2020
    Configuration menu
    Copy the full SHA
    40a780e View commit details
    Browse the repository at this point in the history