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

Improve pip-compile with regard to stdout #361

Closed
wants to merge 5 commits into from

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented May 12, 2016

I also had this, but it seems like -o - should be given explicitly?!

Or should -o - be the default for - as input file (which makes sense to me)?
(Done in 31cbab8)

See also 1aa91b5 where --header gets disabled for this.
Should it be still possible to pass in --header to force it, as with pip-comfile --header -?

diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py
index 9b94809..fd075e6 100755
--- a/piptools/scripts/compile.py
+++ b/piptools/scripts/compile.py
@@ -65,10 +65,6 @@ def cli(verbose, dry_run, pre, rebuild, find_links, index_url, extra_index_url,
                                       "the default is {}").format(DEFAULT_REQUIREMENTS_FILE))
         src_files = (DEFAULT_REQUIREMENTS_FILE,)

-    if len(src_files) == 1 and src_files[0] == '-':
-        if not output_file:
-            raise click.BadParameter('--output-file is required if input is from stdin')
-
     if len(src_files) > 1 and not output_file:
         raise click.BadParameter('--output-file is required if two or more input files are given.')

f = stack.enter_context(AtomicSaver(self.dst_file))

for line in self._iter_lines(results, reverse_dependencies, primary_packages):
log.info(line)

Choose a reason for hiding this comment

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

Instead of removing logging, shouldn't the log be going to stderr like most logging does? If it goes to stderr, it wouldn't be interfering with the stdout issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cjerdonek

shouldn't the log be going to stderr like most logging does?

Agreed. Do you want to create an issue for it?
But then this log line is still a bit too verbose (move it to debug? / add a prefix like "writing line"?).

Choose a reason for hiding this comment

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

Sure, I can make an issue. (I would need to confirm first that log does indeed go to stdout.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Done here: #362

@blueyed
Copy link
Contributor Author

blueyed commented Dec 15, 2016

@piotr-dobrogost
Here we are.. ;)

(As for stats / how likely inclusion will be: https://github.com/nvie/pip-tools/pulls?q=is%3Apr+author%3Ablueyed+is%3Aclosed)

@vphilippon vphilippon added the logging Related to log or console output label Nov 24, 2017
@atugushev
Copy link
Member

Close in favour of #727.

@atugushev atugushev closed this May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
logging Related to log or console output
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants