Refactor to psutil #2, added tests #95
Conversation
|
Hi @rsiera, thank you for the PR, I will take a look at it. |
| cluster = db_client.establish_user_defined_connection(instance, clusters) | ||
| except (NotConnectedError, NoPidConnectionError): | ||
| logger.error('failed to acquire details about the database cluster {0}, the server ' | ||
| 'will be skipped'.format(instance)) |
There was a problem hiding this comment.
Not a blocker, but this is really not an improvement over the old version. Error messages should not be broken into multiple lines for grep-ability.
| logger.error('duplicate connection options detected for databases {0} and {1}, ' | ||
| 'same pid {2}, skipping {0}'.format(instance, duplicate_instance, pid)) | ||
| pgcon.close() | ||
| raise DuplicatedConnectionError |
There was a problem hiding this comment.
Not sure why raise this as an error if all we do is ignore it?
|
Wow, this is still a lot of changes. Unfortunately, it takes more time to review this than I can allocate currently. |
|
Hello guys, can we back to this PR and finish the review process of that, so I can fix/apply changes ? I'm aware that it's quite big PR but these tests should make the project more stable and editable. + it finally solves #60 ;) |
|
@rsiera sorry, yes, that's a lot of code. Let me try to give it another shot in the upcoming days. |
4 participants
Issue: #60
Second part of refactoring to psutil. Includes tests and separating logic into classes which try not to break single object responsibility principle. This commit is preparation for last PR, which finally introduces psutil and makes pg_view platform independent.