Initial Version #2
Conversation
joaodaher
force-pushed
the
feature/api-models
branch
from
8e3817c to
fa6e7fa
Compare
joaodaher
force-pushed
the
feature/api-models
branch
from
fa6e7fa to
b2f4462
Compare
.travis.yml
Outdated
| dist: xenial | ||
| sudo: true | ||
| python: | ||
| - "3.6" |
| @property | ||
| def duration(self): | ||
| nanoseconds = self._get('Duration') | ||
| return timedelta(seconds=nanoseconds / 1000 / 1000 / 1000) |
joaodaher
force-pushed
the
feature/api-models
branch
from
b2f4462 to
c2b9a1b
Compare
There was a problem hiding this comment.
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.
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.
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.
what is this? why I cannot refresh the same model if _detailed is True?
There was a problem hiding this comment.
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.
I think this should test the returned value.
| @property | ||
| def duration(self): | ||
| nanoseconds = self._get('Duration') | ||
| return timedelta(seconds=nanoseconds / 1000 / 1000 / 1000) |
|
|
||
| @property | ||
| def value(self): | ||
| return self._get('value') |
There was a problem hiding this comment.
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.