Macos support

[rsiera]

Implementation of #60 - Support for OSX(and other platforms). Honestly I didn't expect so many changes, but in order to replace getting data from /proc file with psutil functions I had to do some refactor, write tests. Dividing this script to modules, separating some common logic to classes makes it much easier. Please review and inform me what do you think about that ? I use this script very often and have some ideas what could be useful and I would be willing to implement some new features. I hope that these changes and especially tests, made this code easier to maintain and improve.

[mayureshnw]

Hi , This looks like its under development. Any particular reason why there has been no conversation over this ? Also, would like to help with the port

[hjacobs]

Stupid question: why would we want to support pg_view on Mac OS? It's hard to test whether the support will continue to work for Linux developers like myself (no legal way of running Mac OS...) and nobody (?) is running production workloads on Mac OS (?).

[rsiera]

I can see some reasons:

1. It's not only OS X support, current implementation is pretended to work on every system which is supported by psutil - so from they docs: Linux, Windows, OSX, Sun Solaris, FreeBSD, OpenBSD and NetBSD.
2. There was a ticket #60 which I also voted for, because I'm used to using this tool during development, debugging process, and currently I am involved in project on OS X. No vagrant, no docker, there are some graphic tools but I would like to have simple tools which I can also use on server.
3. This logic - parsing /proc files, gathering this kind of system informations has been already implemented in psutil, and psutil does it better than pg_view. It handles some quirks like different file formats for different kernel version etc which implementation in pg_view doesn't handle. From code point of view it's better to remove this code from pg_view side and allow tool which is designed to these operations (psutil) to do that.

[alexeyklyukin]

Hi, thanks for the pull request! I hope I will get to reviewing it at some point in the near future.

[alexeyklyukin]

Hi,

I took a look at your changes and tried to run it on both Linux on OS X.

On Linux, I had to change the requirements to avoid asking for the very latest psycopg2.6.2, as a lot of systems ship with older version of psycopg2, as far as 2.4.5 is known to works normally with pg_view.

On OS X, I couldn't run it until I changed the code that calculates the diffs for PostgreSQL process:

 def process_sort_key(process):
-    return process.get('age', maxsize)
+    return process.get('age', maxsize) or maxsize

On a busy Linux system I observed pg_view crashing every minute or so with NoSuchProcess: psutil.NoSuchProcess no process found with pid 13105

I think this problem is not specific to Linux, since psutil is used for both platforms, I just don't have a busy OS X host to check :-) I think it tries to collect statistics for the process that is already gone and doesn't handle this exception gracefully. The original code handled such exceptions and continued working (look, for example, at _read_proc).

I like your restructuring of the code into multiple modes, although I think some smaller files (those, describing the constants, factories, exceptions) could be combined into a single one, something like "utils.py" or "misc.py".

Unfortunately, in the current state the code is impossible to review, as it's hard to tell the pieces of the code that were changed from those that were moved from pg_view.py into smaller files. I'd propose to split this pull request into several, perhaps, the one that does the code reshuffling without changing the actual code, another one that changes the code without introducing psutils and the third one that adds psuitils. What do you think?

[CyberDem0n]

I've also looked on it and tried to run on my home laptop where I have a python compiled without ipv6 support and it was failing because of that.

Besides problems with process_sort_key and ipv6 there are come problems with python3 compatibility which are not that hard to fix.

I think it is already too late to split this pull request into multiple, because usually nobody likes to do the same job twice.

May be we should merge it into separate branch, apply all obvious fixes, do some further refactoring and when it would be mature enough merge it to master?

[alexeyklyukin]

@CyberDem0n I think we could do this, but it would take much more time to review and get it in shape. @rsiera what do you think? Are you willing to submit this pull requests in chunks, so that we can review it quickly, or should we take it and treat like a black box, figuring out what has changed and how it works anew?

[rsiera]

Although it would be more work for me, I totally agree with @alexeyklyukin. In current state it's very difficult to make a good code review, because this PR contains refactoring + adding psutils at once + it's quite big. Your approach with dividing it to 3 PR would be better. So I'll prepare following pull request:

1. Splitting code into separate modules without changes
2. Changes + tests
3. psutils + tests

I will ask you for code-review after every single step, and it should be much easier to review and to achieve working version with psutil support. Thanks for feedback and pointing out issues, I have tested new version on Mac OS + VM with Linux but didn't check these cases, I will take take a look at these issues during second step. I had to mix sth up when handling exceptions ;-)

[alexeyklyukin]

Makes sense to me, thank you. This PR is closed.