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

[Michael Santana]

On Thu, Aug 12, 2021 at 9:39 AM Aaron Conole <aconole at redhat.com> wrote:
>
> 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.
Ok, that's better. We also wouldn't need to update the existing github
entries. I will send V2 with your proposed changes. Will also squash
1/3 and 2/3
>
> 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
>