worker and master splited #8

Closed
cis-muzahid wants to merge 1 commits from cis-muzahid/buildbot_multibuild:kivy-test into master
Member
No description provided.
PeterSurda requested changes 2022-02-17 08:04:30 +01:00
@ -0,0 +2,4 @@
from os.path import exists, isfile, join
import requests
import re
from buildbot.plugins import util
Owner

this is probably here only because of @util.renderer, which shouldn't be on the worker

this is probably here only because of `@util.renderer`, which shouldn't be on the worker
Author
Member

Removed from worker

Removed from worker
Owner

also remove from .renderers import *

also remove `from .renderers import *`
PeterSurda marked this conversation as resolved
@ -0,0 +71,4 @@
return results
def _get_dockerfile_contents(props):
Owner

can't access props, instead the jobname variable needs to be passed from master

can't access `props`, instead the `jobname` variable needs to be passed from master
Author
Member

Also passed os_codename instead of util.Interpolate("%(prop:os_codename)s")

Also passed os_codename instead of `util.Interpolate("%(prop:os_codename)s")`
PeterSurda marked this conversation as resolved
@ -0,0 +86,4 @@
}[util.Interpolate("%(prop:os_codename)s")]
@util.renderer
Owner

this can't be on a worker, because of the decorator
however, it needs to worker differently. It needs to return getProperty('dockerfile') or soemthing like that.

this can't be on a worker, because of the decorator however, it needs to worker differently. It needs to return `getProperty('dockerfile')` or soemthing like that.
PeterSurda marked this conversation as resolved
@ -0,0 +91,4 @@
return _get_dockerfile_contents(props)
def trigger_child_hooks(buildbotUrl: str, os_codename: str, repository, branch, jobname, directory=".buildbot"):
Owner
  • buildbotUrl: not sure, perhaps needs to be passed from master
  • os_codename: probably there is a python library that does this
  • repository, branch, jobname: master needs to pass this data to worker
  • direcotry: can remain as it is as it's only for testing, we don't really need this
- buildbotUrl: not sure, perhaps needs to be passed from master - os_codename: probably there is a python library that does this - repository, branch, jobname: master needs to pass this data to worker - direcotry: can remain as it is as it's only for testing, we don't really need this
PeterSurda marked this conversation as resolved
@ -0,0 +108,4 @@
# make a post request
request_data["properties"] = {
"dockerfile": get_dockerfile_contents(
Owner

"dockerfile": _get_dockerfile_contents(

`"dockerfile": _get_dockerfile_contents(`
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -17,56 +17,8 @@ import re
from buildbot.plugins import steps, util
from .lib.renderers import *
from .lib.worker_multibuild import trigger_child_hooks
Owner

also only worker

also only worker
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -68,4 +22,2 @@
path =".buildbot"
def list_jobs(directory=".buildbot"):
Owner

needs to be on the worker

needs to be on the worker
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -149,3 +59,3 @@
)
)
Owner

?

?
PeterSurda marked this conversation as resolved
PeterSurda requested changes 2022-02-18 08:36:25 +01:00
multibuild.py Outdated
@ -81,3 +21,1 @@
results.append(_)
return results
addStep(steps.ShellCommand(
Owner

ok but different location (inside add_child_step)

ok but different location (inside `add_child_step`)
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -135,3 +39,1 @@
"jobname": jobname,
}
requests.post(request_url, headers=request_headers, data=request_data)
return _get_dockerfile_contents(props, props.getProperty('jobname', default=None), util.Interpolate("%(prop:os_codename)s"))
Owner

return props.getProperty('dockerfile')

`return props.getProperty('dockerfile')`
PeterSurda marked this conversation as resolved
PeterSurda requested changes 2022-02-21 07:38:38 +01:00
@ -0,0 +69,4 @@
return results
def _get_dockerfile_contents(props, jobname, os_codename):
Owner

no props

no `props`
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -135,3 +34,1 @@
"jobname": jobname,
}
requests.post(request_url, headers=request_headers, data=request_data)
return _get_dockerfile_contents(props, props.getProperty('jobname', default=None), util.Interpolate("%(prop:os_codename)s"))
Owner

no calling _get_dockerfile_contents, just access props

no calling `_get_dockerfile_contents`, just access `props`
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -171,1 +67,4 @@
Add a step to the download, build and test factory
"""
build_factory.addStep(steps.ShellCommand(
Owner

upon reflection, this BuildStep needs to go to the parent, not child

upon reflection, this `BuildStep` needs to go to the parent, not child
PeterSurda marked this conversation as resolved
cis-muzahid force-pushed kivy-test from 17bc49bafc to d3f4e3574e 2022-02-21 10:50:47 +01:00 Compare
PeterSurda requested changes 2022-02-22 08:27:00 +01:00
PeterSurda left a comment
Owner

Also let's just assume os_codename is bionic and make it configurable later.

Also let's just assume os_codename is `bionic` and make it configurable later.
@ -0,0 +16,4 @@
ty = "/change_hook/base"
path =".buildbot"
dockerfile_extra_contents_focal = """
Owner

make this into a dict

make this into a dict
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -135,3 +34,1 @@
"jobname": jobname,
}
requests.post(request_url, headers=request_headers, data=request_data)
return _get_dockerfile_contents(props.getProperty('jobname', default=None), util.Interpolate("%(prop:os_codename)s"))
Owner

return props.getProperty('dockerfile')

We already have this information.

`return props.getProperty('dockerfile')` We already have this information.
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -144,1 +44,4 @@
command=["sudo", "wget", "-O", "https://git.bitmessage.org/Bitmessage/buildbot_multibuild/raw/branch/master/lib/worker_multibuild.py", "worker_multibuild.py"]
))
build_factory.addStep(
Owner

os_codename should be per-Dockerfile. We don't really need os_codename in the parent, only in the child.

The Dockerfiles will begin with, e.g. FROM ubuntu:bionic .... So the parent will then extract the codename from that, and based on that append the corresponding buildbot's setup and entrypoint.

`os_codename` should be per-Dockerfile. We don't really need `os_codename` in the parent, only in the child. The `Dockerfile`s will begin with, e.g. `FROM ubuntu:bionic ...`. So the parent will then extract the codename from that, and based on that append the corresponding buildbot's setup and entrypoint.
PeterSurda marked this conversation as resolved
PeterSurda reviewed 2022-02-22 08:31:06 +01:00
multibuild.py Outdated
@ -93,54 +29,21 @@ def find_artifacts(directory="out"):
return join(directory, _)
@util.renderer
Owner

we don't need this function

we don't need this function
PeterSurda marked this conversation as resolved
PeterSurda reviewed 2022-02-23 07:48:07 +01:00
@ -0,0 +76,4 @@
with open(join(path + jobname), "r") as file:
contents = file.read()
# remove any line containing FROM or RUN keywords
re.match(r"(?m)^(FROM|RUN).*$", "", contents)
Owner

the check should be reversed. Instead of removing lines containing FROM or RUN, it should remove the other lines.

the check should be reversed. Instead of removing lines containing FROM or RUN, it should remove the other lines.
PeterSurda marked this conversation as resolved
PeterSurda reviewed 2022-02-23 07:49:36 +01:00
multibuild.py Outdated
@ -151,1 +41,4 @@
build_factory.addStep(steps.ShellCommand(
name="download worker",
command=["sudo", "wget", "-O", "https://git.bitmessage.org/Bitmessage/buildbot_multibuild/raw/branch/master/lib/worker_multibuild.py", "worker_multibuild.py"]
Owner

no need for sudo, however we need a different directory

no need for sudo, however we need a different directory
PeterSurda marked this conversation as resolved
PeterSurda requested changes 2022-02-23 07:51:02 +01:00
PeterSurda left a comment
Owner

Update path to multibuild (downloading and executing it). It shouldn't mix with the repository that's being processed.

Update path to `multibuild` (downloading and executing it). It shouldn't mix with the repository that's being processed.
cis-muzahid force-pushed kivy-test from a40d498533 to 483db0d732 2022-02-23 11:11:26 +01:00 Compare
PeterSurda requested changes 2022-02-24 08:08:00 +01:00
@ -0,0 +79,4 @@
# accept any line containing FROM or RUN keywords
# re.match(r"(?m)^(FROM|RUN).*$", contents)
for i in range(len(contents)):
Owner
inside_allowed_command = False
for line in contents:
    # maybe add newlines when appending line
    if re.match(r"(?m)^(FROM|RUN).*$", line):
        inside_allowed_command = True
    if inside_allowed_command:
        ret += line
        if not re.match(r"\\\w*$", line):
            inside_allowed_command = False
``` inside_allowed_command = False for line in contents: # maybe add newlines when appending line if re.match(r"(?m)^(FROM|RUN).*$", line): inside_allowed_command = True if inside_allowed_command: ret += line if not re.match(r"\\\w*$", line): inside_allowed_command = False ```
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -151,1 +41,4 @@
build_factory.addStep(steps.ShellCommand(
name="download worker",
command=["sudo", "wget", "-O", "https://git.bitmessage.org/Bitmessage/buildbot_multibuild/raw/branch/master/lib/worker_multibuild.py", "worker_multibuild.py"]
Owner

maybe into ~/.local/bin/ (also needs to create the directory if not existant)

maybe into `~/.local/bin/` (also needs to create the directory if not existant)
Author
Member

I am using --force-directories for crete folder if not exists

I am using ```--force-directories``` for crete folder if not exists
PeterSurda requested changes 2022-02-25 07:43:25 +01:00
@ -0,0 +85,4 @@
if inside_allowed_command:
res += line
l = line.strip()
if l.endswith("\\") or l.endswith("\\\n"):
Owner
  • \n probably not needed due to strip
  • remove continue and revert conditional
- `\n` probably not needed due to `strip` - remove `continue` and revert conditional
PeterSurda marked this conversation as resolved
multibuild.py Outdated
@ -151,1 +41,4 @@
build_factory.addStep(steps.ShellCommand(
name="download worker",
command=["sudo", "wget", "-O", "https://git.bitmessage.org/Bitmessage/buildbot_multibuild/raw/branch/master/lib/worker_multibuild.py", ".local/bin/worker_multibuild.py"]
Owner

no sudo

no `sudo`
Owner

also maybe
os.path.join(os.getenv['HOME'], '.local/bin/worker_multibuild.py')

also maybe `os.path.join(os.getenv['HOME'], '.local/bin/worker_multibuild.py')`
PeterSurda marked this conversation as resolved
PeterSurda requested changes 2022-02-25 07:44:44 +01:00
multibuild.py Outdated
@ -169,2 +71,3 @@
def add_child_sh_steps(build_factory, directory=".buildbot"):
"""
Add a step to the build factory
Add a step to the download, build and test factory
Owner

please same indent as elsewhere

please same indent as elsewhere
PeterSurda marked this conversation as resolved
cis-muzahid force-pushed kivy-test from 8e6ab5eea0 to c151641ebe 2022-02-25 08:17:23 +01:00 Compare
PeterSurda requested changes 2022-02-28 07:48:09 +01:00
PeterSurda left a comment
Owner

Please finish.

Please finish.
@ -135,3 +32,1 @@
"jobname": jobname,
}
requests.post(request_url, headers=request_headers, data=request_data)
# @util.renderer
Owner

remove

remove
PeterSurda marked this conversation as resolved
@ -152,0 +44,4 @@
command=["wget", "-O", "https://git.bitmessage.org/Bitmessage/buildbot_multibuild/raw/branch/master/lib/worker_multibuild.py", join(getenv['HOME'], '.local/bin/worker_multibuild.py')]
))
# build_factory.addStep(
Owner

remove

remove
PeterSurda marked this conversation as resolved
cis-muzahid closed this pull request 2022-02-28 13:57:17 +01:00
cis-muzahid requested review from PeterSurda 2022-02-28 15:38:08 +01:00

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Bitmessage/buildbot_multibuild#8
No description provided.