Drop version field from xxxPGCtl classes#158
Conversation
The value for this field was set in `init`, which required a call to the `pg_ctl --version` command. We may want to instanciate a class without executing `pg_ctl`. Since this version field is not really useful, we drop it. This is a breaking change.
|
The other option is to simply drop the |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #158 +/- ##
=======================================
Coverage 97.60% 97.61%
=======================================
Files 10 10
Lines 1211 1214 +3
=======================================
+ Hits 1182 1185 +3
Misses 29 29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return parse_control_data(r.splitlines()) | ||
|
|
||
| @property | ||
| def version(self) -> int: |
There was a problem hiding this comment.
I was wondering if it would be a good idea or not if it was cached. Hence the inaccurate commit message.
There was a problem hiding this comment.
In fact, for the async version it's already quite weird to query a property with await because in most common cases, properties are not expected to involve complex computations, especially not I/O. At least, in that case (in contrast with the sync case), this is explicit. So I think it should not be a property at all; rather a method, which itself may be cached. Or we can drop it if we don't use it as you suggest in your other comment.
There was a problem hiding this comment.
Let's drop it.
PR updated.
pgiraud
force-pushed
the
ctl_version_property
branch
from
9897afd to
f6f096b
Compare
pgiraud
changed the title
dlax
left a comment
dlax
left a comment
There was a problem hiding this comment.
If someone appears to use this, they will hopefully complain and we may reintroduce this as a method.
3 participants
No description provided.