OCA / interface-github

Tools to interact with github from Odoo
GNU Affero General Public License v3.0
46 stars 77 forks source link

10.0 add GitHub connector GitHub connector odoo #2

Closed legalsylvain closed 6 years ago

legalsylvain commented 7 years ago

Full description with screenshots :

https://github.com/grap/interface-github/blob/10.0_ADD_github_connector_github_connector_odoo/github_connector/README.rst

https://github.com/grap/interface-github/blob/10.0_ADD_github_connector_github_connector_odoo/github_connector_odoo/README.rst

CC : @jgrandguillaume, @sbidoul.

hbrunn commented 7 years ago

@legalsylvain what's your plan here? I'm going to do this migration covered by our contract with the OCA

hbrunn commented 7 years ago

(I don't want to discourage cooperation of course, but if A can do it in contract time, and B must do it in the scarce community time, most preferable distribution seems obvious to me)

legalsylvain commented 7 years ago

Hi @hbrunn. I talked yesterday with @sbidoul who asked me to work for porting this module. I so do it.

Let's go talking by email to see how to join the work, if it's possible.

legalsylvain commented 7 years ago

Question: should we split the module github_connector_odoo in 2? github_connector_odoo for the generic part github_connector_odoo_oca for the part related to OCA (vertical solutions etc.) @sbidoul @jgrandguillaume

For vertical solution. I don't think so. odoo type is not specific to OCA. If you create a custom odoo module for a localization, you'll name it l10n-xxx for sure. (it's an odoo convention). And as it's a OCA module, we could assume that if users use it, they will follow Odoo / oca conventions. (for "connector-" for exemple)

elicoidal commented 7 years ago

For vertical solution. I don't think so. odoo type is not specific to OCA. If you create a custom odoo module for a localization, you'll name it l10n-xxx for sure. (it's an odoo convention). And as it's a OCA module, we could assume that if users use it, they will follow Odoo / oca conventions. (for "connector-" for exemple)

@legalsylvain this is true for the localization. The rest is OCA specific. Anyway this is a nice-to-have from my perspective.

RoelAdriaans commented 7 years ago

Issue with the current code, a team sync does not work.

Backtrace:
2017-10-02 16:21:02,710 8521 INFO oca odoo.addons.github_connector.models.github: Calling https://api.github.com/orgs/B-Informed
2017-10-02 16:21:02,713 8521 INFO oca requests.packages.urllib3.connectionpool: Starting new HTTPS connection (1): api.github.com
2017-10-02 16:21:03,616 8521 INFO oca odoo.addons.github_connector.models.github: Calling https://api.github.com/orgs/B-Informed/members?per_page=30&page=1
2017-10-02 16:21:03,618 8521 INFO oca requests.packages.urllib3.connectionpool: Starting new HTTPS connection (1): api.github.com
2017-10-02 16:21:04,275 8521 INFO oca odoo.addons.github_connector.models.github: Calling https://api.github.com/orgs/B-Informed/repos?per_page=30&page=1
2017-10-02 16:21:04,277 8521 INFO oca requests.packages.urllib3.connectionpool: Starting new HTTPS connection (1): api.github.com
2017-10-02 16:21:05,000 8521 INFO oca odoo.addons.github_connector.models.github: Calling https://api.github.com/orgs/B-Informed/teams?per_page=30&page=1
2017-10-02 16:21:05,002 8521 INFO oca requests.packages.urllib3.connectionpool: Starting new HTTPS connection (1): api.github.com
2017-10-02 16:21:06,129 8521 ERROR oca odoo.http: Exception during JSON request handling.
Traceback (most recent call last):
  File "/home/roel/odoo/oca/odoo/odoo/http.py", line 640, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/home/roel/odoo/oca/odoo/odoo/http.py", line 677, in dispatch
    result = self._call_function(**self.params)
  File "/home/roel/odoo/oca/odoo/odoo/http.py", line 333, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/home/roel/odoo/oca/odoo/odoo/service/model.py", line 101, in wrapper
    return f(dbname, *args, **kwargs)
  File "/home/roel/odoo/oca/odoo/odoo/http.py", line 326, in checked_call
    result = self.endpoint(*a, **kw)
  File "/home/roel/odoo/oca/odoo/odoo/http.py", line 935, in __call__
    return self.method(*args, **kw)
  File "/home/roel/odoo/oca/odoo/odoo/http.py", line 506, in response_wrap
    response = f(*args, **kw)
  File "/home/roel/odoo/oca/odoo/addons/web/controllers/main.py", line 889, in call_button
    action = self._call_kw(model, method, args, {})
  File "/home/roel/odoo/oca/odoo/addons/web/controllers/main.py", line 877, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/home/roel/odoo/oca/odoo/odoo/api.py", line 689, in call_kw
    return call_kw_multi(method, model, args, kwargs)
  File "/home/roel/odoo/oca/odoo/odoo/api.py", line 680, in call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/home/roel/odoo/oca/interface-github/github_connector/models/abstract_github_model.py", line 174, in button_update_from_github_full
    return self.update_from_github(True)
  File "/home/roel/odoo/oca/interface-github/github_connector/models/abstract_github_model.py", line 195, in update_from_github
    self.full_update()
  File "/home/roel/odoo/oca/interface-github/github_connector/models/github_organization.py", line 95, in full_update
    self.button_sync_team()
  File "/home/roel/odoo/oca/interface-github/github_connector/models/github_organization.py", line 161, in button_sync_team
    data, {'organization_id': organization.id})
  File "/home/roel/odoo/oca/interface-github/github_connector/models/abstract_github_model.py", line 145, in get_from_id_or_create
    return self._create_from_github_data(data, extra_data)
  File "/home/roel/odoo/oca/interface-github/github_connector/models/abstract_github_model.py", line 214, in _create_from_github_data
    vals = self.get_odoo_data_from_github(data)
  File "/home/roel/odoo/oca/interface-github/github_connector/models/github_team.py", line 101, in get_odoo_data_from_github
    data['organization'])
KeyError: 'organization'

Content of the data dict:

{u'repositories_url': u'https://api.github.com/teams/869046/repos', u'members_url': u'https://api.github.com/teams/869046/members{/member}', u'description': None, u'privacy': u'secret', u'url': u'https://api.github.com/teams/869046', u'permission': u'admin', u'id': 869046, u'slug': u'owners', u'name': u'Owners'}

-- Edit: Fixed with: https://github.com/grap/interface-github/pull/5

astirpe commented 7 years ago

I've also got this:


2017-10-02 14:59:42,000 22938 ERROR github1 odoo.http: Exception during JSON request handling.
Traceback (most recent call last):
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/http.py", line 638, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/http.py", line 675, in dispatch
    result = self._call_function(**self.params)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/http.py", line 331, in _call_function
    return checked_call(self.db, *args, **kwargs)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/service/model.py", line 119, in wrapper
    return f(dbname, *args, **kwargs)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/http.py", line 324, in checked_call
    result = self.endpoint(*a, **kw)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/http.py", line 933, in __call__
    return self.method(*args, **kw)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/http.py", line 504, in response_wrap
    response = f(*args, **kw)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/addons/web/controllers/main.py", line 889, in call_button
    action = self._call_kw(model, method, args, {})
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/addons/web/controllers/main.py", line 877, in _call_kw
    return call_kw(request.env[model], method, args, kwargs)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/api.py", line 681, in call_kw
    return call_kw_multi(method, model, args, kwargs)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/odoo-10.0/odoo/api.py", line 672, in call_kw_multi
    result = method(recs, *args, **kwargs)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/oca/github_connector/models/abstract_github_model.py", line 174, in button_update_from_github_full
    return self.update_from_github(True)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/oca/github_connector/models/abstract_github_model.py", line 195, in update_from_github
    self.full_update()
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/oca/github_connector/models/github_organization.py", line 93, in full_update
    self.button_sync_member()
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/oca/github_connector/models/github_organization.py", line 138, in button_sync_member
    partner = partner_obj.get_from_id_or_create(data)
  File "/home/onestein/Pycharm/Gitlab/odoo-10.0/src/oca/github_connector/models/abstract_github_model.py", line 144, in get_from_id_or_create
    data = github_connector.get_by_url(data['url'])
TypeError: get_by_url() takes at least 3 arguments (2 given)
RoelAdriaans commented 7 years ago

@astirpe the get_by_url() takes at least 3 arguments error should be fixed in https://github.com/grap/interface-github/pull/2

hurrinico commented 7 years ago

Tested Locally works fine :+1:

legalsylvain commented 7 years ago

Hi @lasley. Thanks a lot for your review.

I'll work on your remarks Friday.

regards.

legalsylvain commented 7 years ago

@lasley, @astirpe, @elicoidal : changes done.

@sbidoul : I renamed milestone into serie. More understandable, and avoid confusion with other concept like github milestones.

regards.

elicoidal commented 6 years ago

@legalsylvain What is the status on this PR. Could we expect to merge soon?

legalsylvain commented 6 years ago

@elicoidal. For me, I have made all the changes asked by reviewers. @astirpe did'nt updated his review, but I think it's OK.

I think it could be merged.

regards.

astirpe commented 6 years ago

Clicking all around I still see some small bugs, basically are regarding robustness.

If you all agree with an optimistic merge, for me is also ok.

legalsylvain commented 6 years ago

Quite annoyed by your last 2 comments. ;-) I prefer to check that two points before merging. I try to do it this week (or week-end). regards, and thanks for your review.

sbidoul commented 6 years ago

Fantastic job you did here Sylvain!

Except a problem with duplicate partners with the same github login, everything seems to work pretty well.

I've installed the latest version of this PR and enabled the 4 crons on the test v10 instance (https://odoo10.odoo-community.org). Let's seen how it goes over the night.

legalsylvain commented 6 years ago

@sbidoul : Thanks ! @astirpe : Just fixed in the last f3247d6 commit the two bugs you mentioned.

legalsylvain commented 6 years ago

3 :+1:. @sbidoul, if your pre-production test is OK on new instance, I think we can merge. Regards.

sbidoul commented 6 years ago

Yes sure, please merge. We can still fix any remaining issue later.

elicoidal commented 6 years ago

Good job!!!!!!

RoelAdriaans commented 6 years ago

:+1: :tada: :balloon: :100: