chore: script to automate updates of compatibility test configs for new releases #3541

charleenfei · 2023-05-02T16:26:54Z

Description

Currently, we have had to manually replace release versions with new ones if there is EOL or a security issue with one of the versions, and new releases necessitated the manual insertion of the release version into each compatibility text matrix config file.

This script automates each one of these paths depending on user input. It also creates a backup directory before any of the automations are run in case of something going wrong.

Commit Message / Changelog Entry

chore: script to automate updates of compatibility test configs for new releases

see the guidelines for commit messages. (view raw markdown for examples)

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md).
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards and Go style guide.
Wrote unit and integration tests.
Updated relevant documentation (docs/) or specification (x/<module>/spec/).
Added relevant godoc comments.
Provide a commit message to be used for the changelog entry in the PR description for review.
Re-reviewed Files changed in the Github PR explorer.
Review Codecov Report in the comment section below once CI passes.

.github/compatibility-test-matrices/release-script.sh

colin-axner

Overall LGTM, nice work! I'm not so familiar with bash syntax so I just looked at it from a high level. This seems like a nice step in the right direction for automating maintenance of our compatibility tests

This tool should speed up adding/remove versions and reduce likelyhood of missing a file, but it'd still be nice to eventually autogenerate the matrix from a based on a base set of information. Just throwing the idea out there that a file like transfer-chain-a.json should be the same for all release branch directories with the only difference being that older release branches don't test against newer version. This might be a good place to automate test generation next 🤔

colin-axner · 2023-05-04T08:41:04Z

.github/compatibility-test-matrices/release-script.sh

+cp -R "$directory_path"/* "$backup_dir"
+
+# Step 1: Replace the release version in JSON files
+echo "Enter the release version to replace (ie: v4.4.0, leave empty to skip):"


When do we do this?

Edit: I guess if we have a security issue, but then we'd need to have only a single living release branch for that major version?

yes, if we have security issue. but i guess actually for this situation, the delete script is probably better than a replace script

colin-axner · 2023-05-04T08:41:24Z

.github/compatibility-test-matrices/release-script.sh

+  done
+fi
+
+# Step 2: Add new release version to testing configuration files


Should we have a remove script as well?

.github/compatibility-test-matrices/release-script.sh

DimitrisJim · 2023-06-05T22:21:24Z

.github/compatibility-test-matrices/compat.py

@@ -0,0 +1,101 @@
+#!/usr/bin/python3


Added an alternative in Python. Though bash does the trick, the script seemed like it was doing enough to warrant a language that doesn't make your eyes hurt.

I'm generally open to sticking with bash but find this solution both more readable, more maintainable and less error prone.

Still need to get some feedback from Carlos to make sure the script does what is typically required. Other than that, willing to drop this and go with bash if people prefer that.

I definitely like the idea of this sort of tool existing as a python script instead of a bash one. It would be far easier to maintain.

chatton

Reviewing the python version, I really like the look of this script being in python. I think it would be a lot more maintainable (and even testable if we wanted to go to those lengths)

If we do go forward with the python version, I'd like to see type hints introduced since we'd be starting with a very small amount of code it would be easy to do.

chatton · 2023-06-06T12:04:31Z

.github/compatibility-test-matrices/compat.py

@@ -0,0 +1,101 @@
+#!/usr/bin/python3


I definitely like the idea of this sort of tool existing as a python script instead of a bash one. It would be far easier to maintain.

chatton · 2023-06-06T12:06:17Z

.github/compatibility-test-matrices/compat.py

+def find_json_files(directory, ignores=(".",)):
+    """ Find JSON files in a directory. By default, ignore hidden directories."""
+    for root, dirs, files in os.walk(directory):
+        dirs[:] = (d for d in dirs if not d.startswith(ignores))


I'm not used to this syntax, my guess is that the [:]' is creating a copy or new variable so as to not re-assign the variable dirs` in the for loop. I think this is a little awkward and would be easier to understand if we just used a different variable

i.e.

filtered_dirs = (d for d in dirs if not d.startswith(ignores))

totally ugly, agree there but this in-place assignment of the returned directories allows for skipping any filtered entries when topdown=True. Alternative would be an iteration over the dirs and manual del of the correct indices.

In general, ignores was added based on the initial bash script considering a .backup dir being present. Since I haven't added that in (thinking a git restore could do the trick) this could also be removed).

oh I see so we're actually modifying the dirs that we're already iterating over

chatton · 2023-06-06T12:06:28Z

.github/compatibility-test-matrices/compat.py

+        dirs[:] = (d for d in dirs if not d.startswith(ignores))
+        for file_ in files:
+            if file_.endswith(".json"):
+                yield os.path.join(root, file_)


love the generator usage 👍

chatton · 2023-06-06T12:14:20Z

.github/compatibility-test-matrices/compat.py

+
+def has_release_version(json_file, keys, version):
+    """ Check if the json file has the version in question. """
+    return any(version in row for row in (json_file[key] for key in keys))


this is just a personal preference, but I like keeping list comprehensions as shallow as possible. I find it can be difficult to read if you're not very experienced with python.

What about just

def has_release_version(json_file, keys, version): """ Check if the json file has the version in question. """ values = (json_file[key] for key in keys) return any(version in row for row in values)

the flattening in the any(version in row for row in values) is not necessarily intuitive at first.

but don't you know that one-liners always win? Will split up, serves no real point.

one liners win until someone else needs to modify it 😄

chatton · 2023-06-06T12:15:27Z

.github/compatibility-test-matrices/compat.py

+    for row in (json_file[key] for key in keys):
+        if recent not in row:
+            continue        
+        if op == "add":


maybe these can be constants? OP_ADD, OP_REMOVE etc.

chatton · 2023-06-06T12:17:00Z

.github/compatibility-test-matrices/compat.py

+    """ Check if the json file has the version in question. """
+    return any(version in row for row in (json_file[key] for key in keys))
+
+def update_version(json_file, keys, args):


technically handled in the parse args, code, but maybe we can raise a ValueError if the operation is unknown in this function too just to be safe and explode when bad inputs are used.

chatton · 2023-06-06T12:17:56Z

.github/compatibility-test-matrices/compat.py

+    require_version(args)
+    return args
+
+def main(args):


one thing I noticed was that when we run the script giving an version that doesn't exists, it executes successfully but is just silent. I think it would be no harm either logging that the version wasn't found, or raise a ValueError with the input not having been found.

yeah, can see the value in that

DimitrisJim · 2023-06-06T14:56:50Z

.github/compatibility-test-matrices/compat.py

 # Toggle if required. Indent = 4 matches jq.
-DUMP_ARGS = {"indent": 4, "sort_keys": False, "ensure_ascii": False}
+DUMP_ARGS: Dict[Any, Any] = {


unfortunate but mypy (the type checker for the uninitiated) throws a big hissy-fit here. Typing **kwargs in general are a pain iirc.

- Make script executable - Search in $PATH for bash - Exit on uninitialized vars, non-zero exit status in pipelines - Order the releases - Pretty print output.

chatton

LGTM thank you so much @charleenfei & @DimitrisJim , this will make the release process a lot simpler 🥇

One final nit, compat.py is a bit ambiguous, maybe we could name the file update_compatibility_tests.py to be more explicit, not a blocker at all though.

chatton · 2023-06-14T07:46:51Z

scripts/compat.py

+# Suggestions for recent and new versions.
+SUGGESTION: str = "for example, v4.3.0 or 4.3.0"
+# Supported Operations.
+Operation = enum.Enum("Operation", ["ADD", "REMOVE", "REPLACE"])


charleenfei · 2023-06-15T09:06:07Z

scripts/update_compatibility_tests.py

+
+def sorter(key: str) -> str:
+    """Since 'main' < 'vX.X.X' and we want to have 'main' as the first entry
+    in the list, we return a version that is considerably large. If ibc-go


charleenfei

tysm for making a much more readable/maintainable version of my hack <3

damiannolan

Awesome stuff! The python code looks pretty clean, its been a while since I looked at any! I think its more maintainable than bash.

LGTM 🚀

damiannolan · 2023-06-20T09:48:34Z

scripts/update_compatibility_tests.py

+def sorter(key: str) -> str:
+    """Since 'main' < 'vX.X.X' and we want to have 'main' as the first entry
+    in the list, we return a version that is considerably large. If ibc-go
+    reaches this version I'll wear my dunce hat and go sit in the corner.


I will also eat my shorts on live stream

At our current pace of 3 major releases a year we can get there in 33330 years. Think about the poor person on whose face this time bomb will explode, man.

a quantum commit in the year 35353 "F*** you, DimitrisJim."

crodriguezvega

Awesome work, @DimitrisJim! Great idea and initial implementation, @charleenfei!

crodriguezvega · 2023-06-20T10:18:43Z

scripts/update_compatibility_tests.py

+def sorter(key: str) -> str:
+    """Since 'main' < 'vX.X.X' and we want to have 'main' as the first entry
+    in the list, we return a version that is considerably large. If ibc-go
+    reaches this version I'll wear my dunce hat and go sit in the corner.


At our current pace of 3 major releases a year we can get there in 33330 years. Think about the poor person on whose face this time bomb will explode, man.

charleenfei requested review from colin-axner, AdityaSripal, crodriguezvega, damiannolan, chatton and DimitrisJim as code owners May 2, 2023 16:26

DimitrisJim reviewed May 3, 2023

View reviewed changes

.github/compatibility-test-matrices/release-script.sh Outdated Show resolved Hide resolved

damiannolan reviewed May 4, 2023

View reviewed changes

.github/compatibility-test-matrices/release-script.sh Outdated Show resolved Hide resolved

DimitrisJim reviewed May 4, 2023

View reviewed changes

.github/compatibility-test-matrices/release-script.sh Outdated Show resolved Hide resolved

colin-axner reviewed May 4, 2023

View reviewed changes

DimitrisJim reviewed May 4, 2023

View reviewed changes

.github/compatibility-test-matrices/release-script.sh Outdated Show resolved Hide resolved

srdtrk reviewed May 5, 2023

View reviewed changes

.github/compatibility-test-matrices/release-script.sh Outdated Show resolved Hide resolved

DimitrisJim reviewed May 22, 2023

View reviewed changes

.github/compatibility-test-matrices/release-script.sh Outdated Show resolved Hide resolved

charleenfei marked this pull request as draft May 23, 2023 14:07

DimitrisJim force-pushed the charly/release_cutting_script branch 3 times, most recently from 1263b1b to 365a24f Compare June 5, 2023 22:17

DimitrisJim reviewed Jun 5, 2023

View reviewed changes

DimitrisJim force-pushed the charly/release_cutting_script branch 4 times, most recently from 9211f01 to 0296228 Compare June 5, 2023 22:40

chatton reviewed Jun 6, 2023

View reviewed changes

DimitrisJim reviewed Jun 6, 2023

View reviewed changes

charleenfei and others added 6 commits June 13, 2023 10:15

v1 script

21ee8d1

add script

1e14855

update script comments

e0fadda

Apply suggestions:

4e01830

- Make script executable - Search in $PATH for bash - Exit on uninitialized vars, non-zero exit status in pipelines - Order the releases - Pretty print output.

wip: Let the snake in.

1f23897

Address some of the feedback.

d9e9802

Remove bash script, add Python script.

9a0f611

DimitrisJim force-pushed the charly/release_cutting_script branch from 5eb664e to 9a0f611 Compare June 13, 2023 07:37

Add more logs that indicate updated/skipped files accordingly.

fdcd976

DimitrisJim marked this pull request as ready for review June 13, 2023 09:16

DimitrisJim force-pushed the charly/release_cutting_script branch from 1310304 to 9708c41 Compare June 13, 2023 09:25

Add some documentation.

52639b2

DimitrisJim force-pushed the charly/release_cutting_script branch from 9708c41 to 52639b2 Compare June 13, 2023 09:30

chatton approved these changes Jun 14, 2023

View reviewed changes

chatton mentioned this pull request Jun 14, 2023

Setup virtual env for any python scripts #3837

Open

3 tasks

DimitrisJim added 3 commits June 14, 2023 11:21

Use a more explicit name.

00d7f41

Merge branch 'main' into charly/release_cutting_script

9dc72a2

Merge branch 'main' into charly/release_cutting_script

fe12925

charleenfei commented Jun 15, 2023

View reviewed changes

Merge branch 'main' into charly/release_cutting_script

84b5d97

damiannolan approved these changes Jun 20, 2023

View reviewed changes

crodriguezvega approved these changes Jun 20, 2023

View reviewed changes

Merge branch 'main' into charly/release_cutting_script

06640ec

DimitrisJim merged commit 2e44243 into main Jun 20, 2023

DimitrisJim deleted the charly/release_cutting_script branch June 20, 2023 11:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: script to automate updates of compatibility test configs for new releases #3541

chore: script to automate updates of compatibility test configs for new releases #3541

charleenfei commented May 2, 2023

colin-axner left a comment

colin-axner May 4, 2023 •

edited

Loading

charleenfei May 4, 2023

colin-axner May 4, 2023

DimitrisJim Jun 5, 2023

chatton Jun 6, 2023

chatton left a comment

chatton Jun 6, 2023

chatton Jun 6, 2023

DimitrisJim Jun 6, 2023

chatton Jun 12, 2023

chatton Jun 6, 2023

chatton Jun 6, 2023

DimitrisJim Jun 6, 2023

chatton Jun 7, 2023 •

edited

Loading

chatton Jun 6, 2023

chatton Jun 6, 2023

chatton Jun 6, 2023

DimitrisJim Jun 6, 2023

DimitrisJim Jun 6, 2023 •

edited

Loading

chatton left a comment

chatton Jun 14, 2023

charleenfei Jun 15, 2023

charleenfei left a comment

damiannolan left a comment

damiannolan Jun 20, 2023

crodriguezvega Jun 20, 2023

DimitrisJim Jun 20, 2023

crodriguezvega left a comment

crodriguezvega Jun 20, 2023

chore: script to automate updates of compatibility test configs for new releases #3541

chore: script to automate updates of compatibility test configs for new releases #3541

Conversation

charleenfei commented May 2, 2023

Description

Commit Message / Changelog Entry

colin-axner left a comment

Choose a reason for hiding this comment

colin-axner May 4, 2023 • edited Loading

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

chatton 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chatton Jun 7, 2023 • edited Loading

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

DimitrisJim Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

chatton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charleenfei left a comment

Choose a reason for hiding this comment

damiannolan 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

crodriguezvega left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colin-axner May 4, 2023 •

edited

Loading

chatton Jun 7, 2023 •

edited

Loading

DimitrisJim Jun 6, 2023 •

edited

Loading