[10.0][MIG] Pingen #124

grindtildeath · 2018-01-11T14:27:14Z

Supersedes : #114

guewen · 2018-01-12T07:37:59Z

pingen/models/pingen.py

-        if not response.ok:
-            raise ConnectionError(
-                "%s: %s" % (response.json['errorcode'],
-                            response.json['errormessage']))


What is the rationale for removing this? I think you should at least use response.raise_for_status(), otherwise you might have no json at all if you receive an HTTP response with another status code than 200

When a connection error really happens, i.e. no internet, a ConnectionError (from python lib requests.exceptions) will be raised by the previous instruction response = method(complete_url, **kwargs) and this code won't be executed at all.
The logic was simply, if there's a response it's not a ConnectionError but an APIError instead.
Maybe I'm missing a use case ?

@grindtildeath are authentication error expected as API errors ?

In case the API key is wrong, here is the reponse we get :

(Pdb) response.json() {u'errorcode': 99400, u'errormessage': u'Your token is invalid or expired', u'error': True}

So it will be catched as an API error and the message will be displayed correctly on pingen.document

yvaucher

Few nitpickings otherwise LGTM

pingen/models/ir_attachment.py

yvaucher · 2018-01-12T10:36:18Z

pingen/models/pingen.py

-        if not response.ok:
-            raise ConnectionError(
-                "%s: %s" % (response.json['errorcode'],
-                            response.json['errormessage']))


@grindtildeath are authentication error expected as API errors ?

pingen/models/pingen_document.py

pingen/README.rst

yvaucher · 2018-02-02T16:58:14Z

@guewen can we merge this, was your question answered ?

yvaucher

Found some more nitpickings related to my previous review with the + in concat strings, not that much an issue but for one where a space is missing.

yvaucher · 2018-11-15T15:00:11Z

pingen/models/pingen_document.py

+                          '\n%s') % (self.name, e)
+        except Exception as e:
+            _logger.exception(
+                'Unexpected Error when updating ' +


Here is another unnecessary +

yvaucher · 2018-11-15T15:00:59Z

pingen/models/pingen_document.py

+                self.pingen_color)
+        except ConnectionError:
+            _logger.exception(
+                'Connection Error when asking for sending ' +


+ to remove

yvaucher · 2018-11-15T15:01:12Z

pingen/models/pingen_document.py

+            raise
+        except APIError:
+            _logger.exception(
+                'API Error when asking for sending ' +


+ to remove

yvaucher · 2018-11-15T15:01:33Z

pingen/models/pingen_document.py

+             'state': 'sendcenter',
+             'post_id': post_id})
+        _logger.info(
+            'Pingen Document %s: asked for' +


+ to remove and warning about missing space

yvaucher · 2018-11-15T15:01:49Z

pingen/models/pingen_document.py

+            raise
+        except APIError:
+            _logger.exception(
+                'API Error when asking for sending ' +


+ to remove

yvaucher · 2020-03-05T15:41:43Z

/ocabot merge

OCA-git-bot · 2020-03-05T15:41:59Z

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 10.0-ocabot-merge-pr-124-by-yvaucher-bump-no, awaiting test results.

Signed-off-by yvaucher

OCA-git-bot · 2020-03-05T15:44:33Z

@yvaucher your merge command was aborted due to failed check(s), which you can inspect on this commit of 10.0-ocabot-merge-pr-124-by-yvaucher-bump-no.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

github-actions · 2021-12-05T12:01:04Z

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

oca-clabot · 2022-03-21T14:52:08Z

Hey @grindtildeath, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla
Here is a list of the users:

a-baghirli a.baghirli@mobilunity.com (no github login found)

Appreciation of efforts,
OCA CLAbot

attachment_id not needed Pingen get_pingen_session fixed

Do not raise ConnectionError if we get a response Use ConnectionError from requests lib instead of overriding it Fix push_to_pingen (writes on state as for cron), remove unneeded loops

on pingen ir.attachment. When multiple module redefine create, write it can lead to TypeError

sebalix · 2022-03-30T10:44:02Z

/ocabot merge nobump

OCA-git-bot · 2022-03-30T10:44:18Z

On my way to merge this fine PR!
Prepared branch 10.0-ocabot-merge-pr-124-by-sebalix-bump-nobump, awaiting test results.

Signed-off-by sebalix

OCA-git-bot · 2022-03-30T10:58:32Z

It looks like something changed on 10.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 10.0-ocabot-merge-pr-124-by-sebalix-bump-nobump, awaiting test results.

OCA-git-bot · 2022-03-30T11:23:24Z

Congratulations, your PR was merged at c9d3377. Thanks a lot for contributing to OCA. ❤️

grindtildeath force-pushed the 10.0_mig_pingen branch 4 times, most recently from af0d8b5 to 860f381 Compare January 11, 2018 16:26

guewen reviewed Jan 12, 2018

View reviewed changes

yvaucher requested changes Jan 12, 2018

View reviewed changes

grindtildeath force-pushed the 10.0_mig_pingen branch 2 times, most recently from 9d0752c to 62366a3 Compare January 12, 2018 14:42

yvaucher approved these changes Jan 17, 2018

View reviewed changes

max3903 added this to the 10.0 milestone Jun 12, 2018

yvaucher requested changes Nov 15, 2018

View reviewed changes

yvaucher approved these changes Mar 5, 2020

View reviewed changes

OCA-git-bot added a commit that referenced this pull request Mar 5, 2020

Merge PR #124 into 10.0

7d3d96e

Signed-off-by yvaucher

grindtildeath force-pushed the 10.0_mig_pingen branch from e2324a9 to 2568fb3 Compare March 5, 2020 16:47

github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Dec 5, 2021

github-actions bot closed this Jan 9, 2022

simahawk reopened this Mar 18, 2022

simahawk added no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Mar 18, 2022

grindtildeath force-pushed the 10.0_mig_pingen branch from 2568fb3 to ecf84b7 Compare March 21, 2022 14:52

grindtildeath and others added 4 commits March 21, 2022 16:05

Fix travis.yml

41d4bda

Pingen migrated to 10.0

f470bb7

attachment_id not needed Pingen get_pingen_session fixed

Finish migration to version 10.0

e3b2f67

Do not raise ConnectionError if we get a response Use ConnectionError from requests lib instead of overriding it Fix push_to_pingen (writes on state as for cron), remove unneeded loops

Add api decorator on create, write

0b9574f

on pingen ir.attachment. When multiple module redefine create, write it can lead to TypeError

Remove unneeded + to concatenate string

d2b16d8

grindtildeath force-pushed the 10.0_mig_pingen branch from ecf84b7 to d2b16d8 Compare March 21, 2022 15:25

OCA-git-bot added a commit that referenced this pull request Mar 30, 2022

Merge PR #124 into 10.0

2afd0f1

Signed-off-by sebalix

OCA-git-bot added the bot is merging ⏳ label Mar 30, 2022

OCA-git-bot merged commit e0775ed into OCA:10.0 Mar 30, 2022

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Mar 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10.0][MIG] Pingen #124

[10.0][MIG] Pingen #124

grindtildeath commented Jan 11, 2018

guewen Jan 12, 2018

grindtildeath Jan 12, 2018

yvaucher Jan 12, 2018

grindtildeath Jan 12, 2018

yvaucher left a comment

yvaucher Jan 12, 2018

yvaucher commented Feb 2, 2018

yvaucher left a comment

yvaucher Nov 15, 2018

yvaucher Nov 15, 2018

yvaucher Nov 15, 2018

yvaucher Nov 15, 2018

yvaucher Nov 15, 2018

yvaucher commented Mar 5, 2020

OCA-git-bot commented Mar 5, 2020

OCA-git-bot commented Mar 5, 2020

github-actions bot commented Dec 5, 2021

oca-clabot commented Mar 21, 2022

sebalix commented Mar 30, 2022

OCA-git-bot commented Mar 30, 2022

OCA-git-bot commented Mar 30, 2022

OCA-git-bot commented Mar 30, 2022

[10.0][MIG] Pingen #124

[10.0][MIG] Pingen #124

Conversation

grindtildeath commented Jan 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yvaucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yvaucher commented Feb 2, 2018

yvaucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yvaucher commented Mar 5, 2020

OCA-git-bot commented Mar 5, 2020

OCA-git-bot commented Mar 5, 2020

github-actions bot commented Dec 5, 2021

oca-clabot commented Mar 21, 2022

sebalix commented Mar 30, 2022

OCA-git-bot commented Mar 30, 2022

OCA-git-bot commented Mar 30, 2022

OCA-git-bot commented Mar 30, 2022