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 of process lock on Windows #1547

Closed
wants to merge 27 commits into from
Closed

Fix of process lock on Windows #1547

wants to merge 27 commits into from

Conversation

ngr-t
Copy link
Contributor

@ngr-t ngr-t commented Feb 14, 2016

The current version of
luigi.lock.getpcmd does not work properly on Windows, because Windows does not have ps command which is used to get command in the function. As a result, locking functionality does not work on Windows.
Fix the function to use wmic command instead of ps when luigi is run on Windows.

@erikbern
Copy link
Contributor

After merging #1530 this has a merge conflict – would appreciate if you can rebase and resubmit! Thanks!

Jason Piper and others added 18 commits February 22, 2016 23:39
Server-side filtering of tasks broke when task ids changed from the string
representation of a Task object to a truncated, cleaned up combination of
parameter values and a partial md5 hash. To fix this, we reconstruct a pretty
id more similar to the old value that was matched against for server-side task
filtering.
Some versions of busybox's ps do not include the paramaters being used.
For example this function failed on Alpine linux.

Tested on base docker images of Ubuntu, and Apline.
Allow a single luigi target to open multiple file system types.  This
can allow for easier local testing, unit test, and greater flexability
of your luigi tasks.
Windows does not have ps command, so current version of
luigi.lock.getpcmd does not work on Windows.
Fix to use wmic instead of ps when luigi is run on Windows.
@ngr-t
Copy link
Contributor Author

ngr-t commented Feb 22, 2016

Thank you. I've done rebase and push (there seems to be conflicts, though...)
I didn't confirmed all tox tests pass, but "$ tox -e docs" (the one that my previous commit didn't passed) passed.

Windows does not have ps command, so current version of
luigi.lock.getpcmd does not work on Windows.
Fix to use wmic instead of ps when luigi is run on Windows.
@erikbern
Copy link
Contributor

I think something went wrong when you rebased. Let me know if you can fix it. Alternatively if it's all messed up let me know and I can create a separate PR

Windows does not have ps command, so current version of
luigi.lock.getpcmd does not work on Windows.
Fix to use wmic instead of ps when luigi is run on Windows.
@ngr-t
Copy link
Contributor Author

ngr-t commented Feb 22, 2016

I've fixed it.

@erikbern
Copy link
Contributor

Something looks completely messed up. I think it's easier if you just restart from master and apply the change manually

@ngr-t
Copy link
Contributor Author

ngr-t commented Feb 23, 2016

Sorry.
I'll do as you suggested.

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.

6 participants