Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Add support for new platform (IBM i) #1564
Conversation
====================================================================== FAIL: psutil.tests.test_contracts.TestFetchAllProcesses.test_fetch_all ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/JGORZINS/psutil/psutil/tests/test_contracts.py", line 356, in test_fetch_all meth(ret, p) File "/home/JGORZINS/psutil/psutil/tests/test_contracts.py", line 399, in create_time self.assertIsInstance(ret, float) AssertionError: 1546917581 is not an instance of <class 'float'>
|
Here is the test output. Like I said, it isn't pretty, but if you agree to merging in the new IBM i module, I'll gladly circle around to clean up the test cases (which I haven't yet figured out how to do), perhaps with the help of @jwoehr. |
|
@giampaolo , would you be amenable to merging this in once we further clean up these test cases? |
|
What's the test status? |
|
The test status is still in pretty sad shape (no work has been done since my posted test results, above). |
|
Just let me have a look at the output first to have an idea of what's implemented and what is not. =) |
|
@giampaolo , I've done zero work on this since the posted results on Aug 5th. |
|
Hello Jesse. Here's an initial review. Other than the inline comments, it looks like this platform is quite different than AIX (it has it's own py and c modules) so I suggest to add a Other than that, there are some errors such as this one that need to be fixed: But let's get to tests later (there's more). |
| @@ -163,7 +163,10 @@ | |||
| PROCFS_PATH = "/proc" | |||
|
|
|||
| elif AIX: | |||
| from . import _psaix as _psplatform | |||
| if os.uname().sysname == 'OS400': | |||
giampaolo
Jul 5, 2020
Owner
This won't work on Python 2 (no namedtuple is returned). What's the value of sys.platform? It's probably better to use that one instead.
This won't work on Python 2 (no namedtuple is returned). What's the value of sys.platform? It's probably better to use that one instead.
| # ENOENT (no such file or directory) gets raised on open(). | ||
| # ESRCH (no such process) can get raised on read() if | ||
| # process is gone in meantime. | ||
| if err.errno in (errno.ENOENT, errno.ESRCH): |
giampaolo
Jul 5, 2020
Owner
You don't seem to read /proc so catching ENOENT should not be necessary
You don't seem to read /proc so catching ENOENT should not be necessary
| class Process(object): | ||
| """Wrapper class around underlying C implementation.""" | ||
|
|
||
| __slots__ = ["pid", "_name", "_ppid", "_cache", "_conn"] |
giampaolo
Jul 5, 2020
Owner
looks like "_conn" is not used
looks like "_conn" is not used
| @@ -0,0 +1,518 @@ | |||
| /* | |||
| * Copyright (c) 2009, Giampaolo Rodola' | |||
| * Copyright (c) 2017, Arnon Yaari | |||
giampaolo
Jul 5, 2020
Owner
Replace Arnon with your name
Replace Arnon with your name
| NULL, | ||
| 0, | ||
| &pid_in_table, | ||
| 1); |
giampaolo
Jul 5, 2020
Owner
Please use 4 spaces indentation. Either this:
int rtv = getprocs64(
dest,
sizeof(struct procentry64),
NULL,
0,
&pid_in_table,
1);
...or this (max 80 chars per line):
int rtv = getprocs64(dest, sizeof(struct procentry64), NULL, 0,
&pid_in_table, 1);
Please use 4 spaces indentation. Either this:
int rtv = getprocs64(
dest,
sizeof(struct procentry64),
NULL,
0,
&pid_in_table,
1);...or this (max 80 chars per line):
int rtv = getprocs64(dest, sizeof(struct procentry64), NULL, 0,
&pid_in_table, 1);| for row in cursor: | ||
| ret = row[0].timestamp() | ||
| cursor.close() | ||
| return ret |
giampaolo
Jul 5, 2020
Owner
perhaps you should use row = next(cursor) and avoid the for loop
perhaps you should use row = next(cursor) and avoid the for loop
|
|
||
| def pid_exists(pid): | ||
| """Check for the existence of a unix pid.""" | ||
| return pid in cext.list_pids() |
giampaolo
Jul 5, 2020
Owner
On UNIX, it's usually enough to do this (faster):
pid_exists = _psposix.pid_exists
On UNIX, it's usually enough to do this (faster):
pid_exists = _psposix.pid_exists| # support for private module import | ||
| if (NoSuchProcess is None or AccessDenied is None or | ||
| ZombieProcess is None): | ||
| raise |
giampaolo
Jul 5, 2020
Owner
you can get rid of this stuff
you can get rid of this stuff
| stdout=subprocess.PIPE, stderr=subprocess.PIPE) | ||
| stdout, stderr = p.communicate() | ||
| if PY3: | ||
| stdout, stderr = [x.decode(sys.stdout.encoding) |
giampaolo
Jul 5, 2020
Owner
use _common.decode()
use _common.decode()
| for x in (stdout, stderr)] | ||
| if "no such process" in stderr.lower(): | ||
| raise NoSuchProcess(self.pid, self._name) | ||
| procfiles = re.findall(r"(\d+): S_IFREG.*\s*.*name:(.*)\n", stdout) |
giampaolo
Jul 5, 2020
Owner
You can probably use re.compile('...') and use it as a private argument of this function (faster).
Grep for "re.compile" to see what I mean.
You can probably use re.compile('...') and use it as a private argument of this function (faster).
Grep for "re.compile" to see what I mean.
For reference, see lengthy discussions in #1415 and #1444. I found it eventually much more sensible to have IBM i as a completely new module than sharing a codestream with AIX.
I'll attach the test output shortly. While there is still plenty of test cases that should be cleaned up, the basic functions seem to work well. This fork has been used by several applications for nearly six months, so there is some assurance of quality.
This new module relies in an external package, but IBM will distribute the package in RPM form as to ensure that does not pose a problem.