[pw-ci] [PATCH 1/3] Prepare git_builds to introduce ci_instance column

[Aaron Conole]

Hi Michael,

Thanks for the work.  Just a nit on the subject, please indicate the
component area this is touching.

  'series_db_lib: Introduce a ci_instance column to git_builds'

Michael Santana <msantana at redhat.com> writes:

> This patch starts adding a label to newer entries that distinctly
> identifies github actions from other CI using the same git_builds
> table.
>
> Unfortunately the git_builds table also contains previous entries that
> do not contain the label. This patch is half the implementation. The
> second half of the implementation would break existing automation jobs
> because entries without the label would be mistakenly skipped. This
> patch is intended to give time to automation systems that implement
> pw-ci with github actions to finish processing entries without the
> ci_instance label. Once enough time has passed and all entries without
> labels are processed then the second part of this patch can be safely
> applied.

I am not sure about this.  We can insert the github_actions ci_instance
tag to rows where it isn't present.  Additionally, I think we should
make that a default for now as well (so that existing CI infrastructure
doesn't break).

That would mean we don't have to stage 1/3 and 2/3.

> Signed-off-by: Michael Santana <msantana at redhat.com>
> ---
>  insert_commit_into_db |  9 +++++++--
>  series_db_lib.sh      | 12 +++++++++++-
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/insert_commit_into_db b/insert_commit_into_db
> index a8dcd3c..f126b79 100755
> --- a/insert_commit_into_db
> +++ b/insert_commit_into_db
> @@ -60,12 +60,17 @@ if [ "X" = "X$repo_name" ]; then
>      shift
>  fi
>  
> +if [ "X" = "X$ci_instance" ]; then
> +    ci_instance="$1"
> +    shift
> +fi
> +
>  if [ "X" = "X$series_id" -o "X" = "X$patch_id" -o "X" = "X$patch_url" -o "X" = "X$patch_name" -o \
> -"X" = "X$sha" -o "X" = "X$pw_instance"  -o "X" = "X$project" -o "X" = "X$repo_name" ]; then
> +"X" = "X$sha" -o "X" = "X$pw_instance"  -o "X" = "X$project" -o "X" = "X$repo_name" -o "X" = "X$ci_instance" ]; then
>      echo "Missing arguments to $0. Nothing pushed to database. Exiting" 1>&2
>      exit 1
>  fi
>  
>  patch_name=$(echo "$patch_name" | sed 's@"@""@g')
>  
> -insert_commit "$series_id" "$patch_id" "$patch_url" "$patch_name" "$sha" "$pw_instance" "$project" "$repo_name"
> +insert_commit "$series_id" "$patch_id" "$patch_url" "$patch_name" "$sha" "$pw_instance" "$project" "$repo_name" "$ci_instance"
> diff --git a/series_db_lib.sh b/series_db_lib.sh
> index c511c36..d65a5ba 100644
> --- a/series_db_lib.sh
> +++ b/series_db_lib.sh
> @@ -104,6 +104,16 @@ gap_sync INTEGER);
>  EOF
>          run_db_command "INSERT INTO series_schema_version(id) values (6);"
>      fi
> +
> +    # 0007 - patch data for Open Build Service and Patchwork sync
> +    run_db_command "select * from series_schema_version;" | egrep '^7$' >/dev/null 2>&1
> +    if [ $? -eq 1 ]; then
> +        sqlite3 ${HOME}/.series-db <<EOF
> +ALTER TABLE git_builds ADD COLUMN ci_instance TEXT;
> +EOF

Right after this ALTER, we can then run:

   UPDATE git_builds SET ci_instance="github_actions..." WHERE ci_instance is NULL;

Something like that ensures all the rows get updated.  I didn't test the
above command, so it needs some regression checking.

I also have a question about the way this will work now.  We will need
to run an insert_commit for each build service we want to use.  Can we
do it differently?

Ex:

   ALTER TABLE git_builds ADD COLUMN obs_sync INTEGER;

And for each entry in the table (by default) we can set obs_sync to 1
(or whatever will mean 'completed').  Then when we run the insert, we
can pass multiple arguments to turn on/off gap_sync and obs_sync
independently.  This also lets us do the recheck a bit easier, I think.

Additionally, even if you keep the existing structure (ci_instance text
field), I think it needs to match the name of the mon script.  I see in
2/3, you have chosen an arbitrary name, but I think this creates
confusion.  If you choose to keep this around, please keep the strings
consistent with each other.  It will help us write the upper layers more
generically.

> +        run_db_command "INSERT INTO series_schema_version(id) values (7);"
> +    fi
> +
>  }
>  
>  function series_db_exists() {
> @@ -365,7 +375,7 @@ function insert_commit() {
>  
>      series_db_exists
>  
> -    echo "INSERT INTO git_builds (series_id, patch_id, patch_url, patch_name, sha, patchwork_instance, patchwork_project, repo_name, gap_sync) VALUES($series_id, $patch_id, \"$patch_url\", \"$patch_name\", \"$sha\", \"$instance\", \"$project\", \"$repo_name\", 0);" | series_db_execute
> +    echo "INSERT INTO git_builds (series_id, patch_id, patch_url, patch_name, sha, patchwork_instance, patchwork_project, repo_name, gap_sync, ci_instance) VALUES($series_id, $patch_id, \"$patch_url\", \"$patch_name\", \"$sha\", \"$instance\", \"$project\", \"$repo_name\", 0, \"$ci_instance\");" | series_db_execute
>  }
>  
>  function get_patch_id_by_series_id_and_sha() {
> -- 
> 2.31.1