Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow beetmover to receive optional exclude patterns #1100

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Allow beetmover to receive optional exclude patterns #1100

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1215e24
Allow beetmover to receive optional exclude patterns
oskirby Dec 4, 2024
a9ea743
style: pre-commit.ci auto fixes [...]
pre-commit-ci[bot] Dec 4, 2024
08f8e3a
Update the beetmover schema
oskirby Dec 9, 2024
bb05034
Actually, update all the schemas
oskirby Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion beetmoverscript/src/beetmoverscript/gcloud.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ async def push_to_releases_gcs(context):

# Weed out RELEASE_EXCLUDE matches, but allow partners specified in the payload
push_partners = context.task["payload"].get("partners", [])
exclude = context.task["payload"].get("exclude", []) + list(RELEASE_EXCLUDE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't a problem now, but I will note that this will subject VPN pushes to the existing RELEASE_EXCLUDE patterns, which could theoretically be a problem in the future. Hopefully we'll have removed this hardcode by the time any such thing comes up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding of the beetmover scripts, that was always the case and I felt that this approach was the best way to minimize the potential for unintended changes in behaviour. Once this PR lands we can probably work towards removing RELEASE_EXCLUDE by migrating the patterns into their associated tasks.

The other way we could have done this would be to use RELEASE_EXCLUDE as the default value for existing tasks that rely on RELEASE_EXCLUDE to omit files, for example:

exclude = context.task["payload"].get("exclude", list(RELEASE_EXCLUDE))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - this is already the case. Sorry for the confusion. I think what you have here is fine, but the above suggestion would be fine as well, if you want to go that route.


for blob_path in candidates_blobs.keys():
if "/partner-repacks/" in blob_path:
Expand All @@ -214,7 +215,7 @@ async def push_to_releases_gcs(context):
)
else:
log.debug("Excluding partner repack {}".format(blob_path))
elif not matches_exclude(blob_path, RELEASE_EXCLUDE):
elif not matches_exclude(blob_path, exclude):
blobs_to_copy[blob_path] = blob_path.replace(candidates_prefix, releases_prefix)
else:
log.debug("Excluding {}".format(blob_path))
Expand Down
4 changes: 3 additions & 1 deletion beetmoverscript/src/beetmoverscript/script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ async def push_to_releases_s3(context):

# Weed out RELEASE_EXCLUDE matches, but allow partners specified in the payload
push_partners = context.task["payload"].get("partners", [])
exclude = context.task["payload"].get("exclude", []) + list(RELEASE_EXCLUDE)

for k in candidates_keys_checksums.keys():
if "/partner-repacks/" in k:
partner_match = get_partner_match(k, candidates_prefix, push_partners)
Expand All @@ -236,7 +238,7 @@ async def push_to_releases_s3(context):
)
else:
log.debug("Excluding partner repack {}".format(k))
elif not matches_exclude(k, RELEASE_EXCLUDE):
elif not matches_exclude(k, exclude):
context.artifacts_to_beetmove[k] = k.replace(candidates_prefix, releases_prefix)
else:
log.debug("Excluding {}".format(k))
Expand Down
36 changes: 36 additions & 0 deletions beetmoverscript/tests/test_gcloud.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from scriptworker.exceptions import ScriptWorkerTaskException

import beetmoverscript.gcloud
from beetmoverscript.utils import get_candidates_prefix, get_releases_prefix

from . import get_fake_valid_task, noop_sync

Expand Down Expand Up @@ -233,6 +234,41 @@ def fake_list_bucket_objects_gcs_same(client, bucket, prefix):
await beetmoverscript.gcloud.push_to_releases_gcs(context)


@pytest.mark.parametrize(
"candidate_blobs,exclude,results",
[
({"foo/bar.zip": "md5hash", "foo/baz.exe": "abcd", "foo/qux.js": "shasum"}, [], ["foo/baz.exe", "foo/qux.js"]),
({"foo/bar.zip": "md5hash", "foo/baz.exe": "abcd", "foo/qux.js": "shasum"}, [r"^.*\.exe$"], ["foo/qux.js"]),
({"foo/bar.zip": "md5hash", "foo/baz.exe": "abcd", "foo/qux.js": "shasum"}, [r"^.*\.exe$", r"^.*\.js"], []),
],
)
@pytest.mark.asyncio
async def test_push_to_releases_gcs_exclude(context, monkeypatch, candidate_blobs, exclude, results):
context.gcs_client = FakeClient()
context.task = get_fake_valid_task("task_push_to_releases.json")

payload = context.task["payload"]
payload["exclude"] = exclude
test_candidate_prefix = get_candidates_prefix(payload["product"], payload["version"], payload["build_number"])
test_release_prefix = get_releases_prefix(payload["product"], payload["version"])

def fake_list_bucket_objects_gcs_same(client, bucket, prefix):
if "candidates" in prefix:
return {f"{prefix}{key}": value for (key, value) in candidate_blobs.items()}
if "releases" in prefix:
return {}

expect_blobs = {f"{test_candidate_prefix}{key}": f"{test_release_prefix}{key}" for key in results}

def fake_move_artifacts(client, bucket_name, blobs_to_copy, candidates_blobs, releases_blobs):
assert blobs_to_copy == expect_blobs

monkeypatch.setattr(beetmoverscript.gcloud, "list_bucket_objects_gcs", fake_list_bucket_objects_gcs_same)
monkeypatch.setattr(beetmoverscript.gcloud, "move_artifacts", fake_move_artifacts)

await beetmoverscript.gcloud.push_to_releases_gcs(context)


def test_list_bucket_objects_gcs():
beetmoverscript.gcloud.list_bucket_objects_gcs(FakeClient(), "foobucket", "prefix")

Expand Down
35 changes: 34 additions & 1 deletion beetmoverscript/tests/test_script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
setup_mimetypes,
)
from beetmoverscript.task import get_release_props, get_upstream_artifacts
from beetmoverscript.utils import generate_beetmover_manifest, is_promotion_action
from beetmoverscript.utils import get_candidates_prefix, get_releases_prefix, generate_beetmover_manifest, is_promotion_action

from . import get_fake_valid_config, get_fake_valid_task, get_test_jinja_env, noop_async, noop_sync

Expand Down Expand Up @@ -70,6 +70,39 @@ def fake_list(*args):
await push_to_releases_s3(context)


# push_to_releases_s3_exclude {{{1
@pytest.mark.parametrize(
"candidates_keys,exclude,results",
[
({"foo.zip": "x", "bar.exe": "y", "baz.js": "z"}, [], ["bar.exe", "baz.js"]),
({"foo.zip": "x", "bar.exe": "y", "baz.js": "z"}, [r"^.*\.exe$"], ["baz.js"]),
({"foo.zip": "x", "bar.exe": "y", "baz.js": "z"}, [r"^.*\.exe$", r"^.*\.js"], []),
],
)
@pytest.mark.asyncio
async def test_push_to_releases_s3_exclude(context, mocker, candidates_keys, exclude, results):
payload = {"product": "devedition", "build_number": 33, "version": "99.0b44", "exclude": exclude}
context.task = {"payload": payload}
test_candidate_prefix = get_candidates_prefix(payload["product"], payload["version"], payload["build_number"])
test_release_prefix = get_releases_prefix(payload["product"], payload["version"])

objects = [{f"{test_candidate_prefix}{key}": candidates_keys[key] for key in candidates_keys}, {}]

expect_artifacts = {f"{test_candidate_prefix}{key}": f"{test_release_prefix}{key}" for key in results}

def check(ctx, _, r):
assert ctx.artifacts_to_beetmove == expect_artifacts

def fake_list(*args):
return objects.pop(0)

mocker.patch.object(boto3, "resource")
mocker.patch.object(beetmoverscript.script, "list_bucket_objects", new=fake_list)
mocker.patch.object(beetmoverscript.script, "copy_beets", new=check)

await push_to_releases_s3(context)


# copy_beets {{{1
@pytest.mark.parametrize("releases_keys,raises", (({}, False), ({"to2": "from2_md5"}, False), ({"to1": "to1_md5"}, True)))
def test_copy_beets(context, mocker, releases_keys, raises):
Expand Down