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

Removed message 'No Instance(s) Available.' in Windows when starting … #2294

Conversation

hirolau
Copy link
Contributor

@hirolau hirolau commented Nov 25, 2017

Description

In Window when starting luigi the message No Instance(s) Available. is shown in the terminal. This is due to the fact that command wmic throws this error if no process with the given pid is found. We really do not care about this error, so this change redirects the error message to nul

The method of redirecting standard error was found here:

https://stackoverflow.com/questions/4507312/how-to-redirect-stderr-to-null-in-cmd-exe

Motivation and Context

The message was confusing to new users and gave no important information.

See for example here: https://stackoverflow.com/questions/40593709/luigi-no-instances-available

Have you tested this? If so, how?

Updated the unit tests to work with Windows, and have run my pipeline without problems.

@hirolau
Copy link
Contributor Author

hirolau commented Nov 25, 2017

Sorry, happened to miss that os was already imported. Is this ok or should I make a totally new branch and create a new request?

@hirolau
Copy link
Contributor Author

hirolau commented Nov 25, 2017

I am a bit lost of why my pull-request would fail the codecov/patch. Any idea what I can do to fix it?

Copy link
Contributor

@Tarrasch Tarrasch left a comment

Choose a reason for hiding this comment

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

LGTM.

I don't know how codecov suddenly started. Ignore it. imo this is good to merge. Thanks!

@hirolau
Copy link
Contributor Author

hirolau commented Apr 2, 2018

So, what is needed to get this one merged?

@Tarrasch Tarrasch merged commit 0d87424 into spotify:master Apr 8, 2018
@Tarrasch
Copy link
Contributor

Tarrasch commented Apr 8, 2018

Thanks for this!

This was referenced Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants