Bitmessage/buildbot-transifex
2
0
Fork 1
Code Issues Pull Requests 1 Packages Projects Releases Wiki Activity

Update webhook and setup #1

Open
shekhar-cis wants to merge 5 commits from shekhar-cis/buildbot-transifex:transifex-webhook into master
pull from: shekhar-cis/buildbot-transifex:transifex-webhook
merge into: Bitmessage:master
Conversation 0 Commits 5 Files Changed 2 +6 -14
1 changed files with 6 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 8091be197d - Show all commits

20
buildbot_transifex/webhook.py
View File

@ -66,16 +66,18 @@ class TransifexHandler(BaseHookHandler):
def process_translation_completed(self, payload, codebase): def process_translation_completed(self, payload, codebase):
PeterSurda commented 2023-01-12 07:36:36 +01:00
Outdated
Review
Copy Link

maybe just raise ValueError("Signature mismatch"), then it will propagate up

maybe just `raise ValueError("Signature mismatch")`, then it will propagate up
changes = [] changes = []
PeterSurda commented 2023-01-09 07:40:38 +01:00
Outdated
Review
Copy Link

return True missing

`return True` missing
👍 1
translated_request = self._transform_variables(payload['project'], payload['resource']) translated_request = self._transform_variables(payload['project'], payload['resource'])
ts = int(time.time())
PeterSurda commented 2023-01-06 07:43:11 +01:00
Outdated
Review
Copy Link

tx_signature isn't a function/method

`tx_signature` isn't a function/method
👍 1
change = { change = {
PeterSurda marked this conversation as resolved Outdated
PeterSurda commented 2023-01-06 07:42:57 +01:00
Outdated
Review
Copy Link

return False

`return False`
👍 1
'author': "buildbot-transifex", 'author': author,
PeterSurda commented 2023-01-09 07:41:20 +01:00
Outdated
Review
Copy Link

don't have weird constants here, rather process is through the map.

don't have weird constants here, rather process is through the map.
PeterSurda commented 2023-01-13 07:33:07 +01:00
Outdated
Review
Copy Link

repository missing

`repository` missing
'branch': translated_request["branch"], 'branch': translated_request["branch"],
PeterSurda commented 2023-01-11 07:35:19 +01:00
Outdated
Review
Copy Link

transifex_to_github_map is an object attribute, no need to pass it around

`transifex_to_github_map` is an object attribute, no need to pass it around
'branch': translated_request["repository"],
PeterSurda marked this conversation as resolved Outdated
PeterSurda commented 2023-01-06 07:43:32 +01:00
Outdated
Review
Copy Link
  • use constant
  • use a default value, then no need for try/except
- use constant - use a default value, then no need for try/except
👍 1
'project': translated_request["project"], 'project': translated_request["project"],
PeterSurda commented 2023-01-09 07:41:48 +01:00
Outdated
Review
Copy Link

error handling missing here

error handling missing here
👍 1
'properties': { 'properties': {
"transifex_language": payload.get("language", "None"), "transifex_language": payload.get("language", "None"),
PeterSurda commented 2023-01-10 08:07:48 +01:00
Outdated
Review
Copy Link

if you use .get with a default value, you can avoid try/except

if you use `.get` with a default value, you can avoid `try`/`except`
"transifex_event": payload.get("event", "None"), "transifex_event": payload.get("event", "None"),
PeterSurda commented 2023-01-11 07:36:19 +01:00
Outdated
Review
Copy Link

language = payload.get('language', "None")

`language = payload.get('language', "None")`
"transifex_project": payload.get("project", "None"), "transifex_project": payload.get("project", "None"),
PeterSurda commented 2023-01-13 07:34:04 +01:00
Review
Copy Link

this should be the randomly generated one, the branch of the PR

this should be the randomly generated one, the branch of the PR
"transifex_resource": payload.get("resource", "None"), "transifex_resource": payload.get("resource", "None"),
PeterSurda marked this conversation as resolved Outdated
PeterSurda commented 2023-01-06 07:43:57 +01:00
Outdated
Review
Copy Link

This should be in the buildbot job, not here. The webhook only transforms data structures and puts them into a database.

  • "tx pull -l language {}".format(options["language"). or something like that
This should be in the buildbot job, not here. The webhook only transforms data structures and puts them into a database. - `"tx pull -l language {}".format(options["language").` or something like that
👍 1
PeterSurda commented 2023-01-12 07:37:33 +01:00
Outdated
Review
Copy Link

this line ('resource': ...) should be remove

this line (`'resource': ...`) should be remove
"transifex_branch": "v0.6" "transifex_branch": "translate_" + payload['language'] + "_" + str(ts)
PeterSurda commented 2023-01-12 07:38:53 +01:00
Outdated
Review
Copy Link

something like translated_request or mapped_request would be more understandable. There is not response coming from transifex.

something like `translated_request` or `mapped_request` would be more understandable. There is not response coming from transifex.
} }
PeterSurda commented 2023-01-11 07:37:05 +01:00
Outdated
Review
Copy Link

not translating anything. There is nothing here that translates transifex payload information into github information.

not translating anything. There is nothing here that translates transifex payload information into github information.
} }
if codebase is not None: if codebase is not None:
@ -83,10 +85,10 @@ class TransifexHandler(BaseHookHandler):
changes.insert(0, change) changes.insert(0, change)
return changes return changes
PeterSurda commented 2023-01-09 07:46:42 +01:00
Outdated
Review
Copy Link

this is a sub-property

this is a sub-property
PeterSurda commented 2023-01-09 07:46:50 +01:00
Outdated
Review
Copy Link

from map

from map
def _transform_variables(self, transifex_project, transifex_resource): def _transform_variables(self, transifex_project):
PeterSurda commented 2023-01-04 08:08:47 +01:00
Outdated
Review
Copy Link
  • no need super
  • needs master
  • doesn't save transifex_dict
  • secret also missing
- no need super - needs `master` - doesn't save `transifex_dict` - `secret` also missing
PeterSurda commented 2023-01-09 07:47:01 +01:00
Outdated
Review
Copy Link

from map

from map
if transifex_project is None: if transifex_project is None:
raise ValueError("Unknown project %s from transifex".format(transifex_project)) raise ValueError("Unknown project %s from transifex".format(transifex_project))
key = "{}/{}".format(transifex_project, transifex_resource) key = transifex_project
_map = self.map[key] _map = self.map[key]
repository = _map["repository"] repository = _map["repository"]
project = re.sub(r'^.*/(.*?)(\.git)?$', r'\1', repository) project = re.sub(r'^.*/(.*?)(\.git)?$', r'\1', repository)
@ -122,16 +124,6 @@ class TransifexHandler(BaseHookHandler):
request.getHeader(_HEADER_SIGNATURE)) request.getHeader(_HEADER_SIGNATURE))
self._verifyTransifexSignature(request, content, rendered_secret, signature, header_signature) self._verifyTransifexSignature(request, content, rendered_secret, signature, header_signature)
event_type = payload.get("event", "None") event_type = payload.get("event", "None")
PeterSurda commented 2023-01-13 07:36:00 +01:00
Outdated
Review
Copy Link

duplicate

duplicate
mapped_request = self._transform_variables(payload['project'], payload['resource'])
change["changes"] = {
"author": author,
"repository": mapped_request["repository"],
"project": mapped_request["project"],
"branch": mapped_request["branch"]
}
log.msg("Received event '{}' from transifex".format(event_type)) log.msg("Received event '{}' from transifex".format(event_type))
codebase = "" codebase = ""
PeterSurda commented 2023-01-12 07:48:32 +01:00
Outdated
Review
Copy Link

since we're raising inside the _verifyTransifexSignature, no need for if here anymore

since we're raising inside the `_verifyTransifexSignature`, no need for `if` here anymore