-
Notifications
You must be signed in to change notification settings - Fork 0
Initial Version #2
base: master
Are you sure you want to change the base?
Conversation
8e3817c
to
fa6e7fa
Compare
fa6e7fa
to
b2f4462
Compare
.travis.yml
Outdated
dist: xenial | ||
sudo: true | ||
python: | ||
- "3.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't you support Python 2? 😱
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because RIP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@property | ||
def duration(self): | ||
nanoseconds = self._get('Duration') | ||
return timedelta(seconds=nanoseconds / 1000 / 1000 / 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: nanoseconds / 1e9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or nanoseconds / (1000 ** 3)
b2f4462
to
c2b9a1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I miss some separation between class methods and instance methods:
models.App.get(pk=666)
app = models.App()
app.get(pk=666)
It could clash some methods (django solves it using the .objects
attribute).
I also see some repetition like:
def user(self):
return self._get('User')
But I'm not sure if it can be optimized.
|
||
|
||
class TestTsuruClient(HTTPPrettyTestMixin, unittest.TestCase): | ||
@httpretty.activate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't possible to use allow_net_connect=False
?
https://httpretty.readthedocs.io/en/latest/api.html#enable
|
||
def setUp(self): | ||
self.patcher = patch('tsuru.client.TsuruClient._URL', self.API_URL) | ||
self.patcher.start() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are not using self.patcher
on tests, right?
So, you can avoid to save patcher
on self
using addCleanup
:
patcher = patch('tsuru.client.TsuruClient._URL', self.API_URL)
patcher.start()
self.addCleanup(patcher.stop)
tsuru/models/base.py
Outdated
raise exceptions.UnexpectedDataFormat() | ||
|
||
def refresh(self): | ||
if not self._detailed: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this? why I cannot refresh the same model if _detailed
is True
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should rename this (refresh
looks like refresh_from_db
on a django model).
def test_list(self): | ||
data = self.sample_list() | ||
with self.patch_get(data=data) as get: | ||
list(models.App.list()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should test the returned value.
@property | ||
def duration(self): | ||
nanoseconds = self._get('Duration') | ||
return timedelta(seconds=nanoseconds / 1000 / 1000 / 1000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or nanoseconds / (1000 ** 3)
|
||
@property | ||
def value(self): | ||
return self._get('value') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we handle the ****
if the value is not public
?
Code Climate has analyzed commit cbc0871 and detected 0 issues on this pull request. View more on Code Climate. |
No description provided.