It could also be done on patchset-created, I don't think we need jenkins for this.

If you do this, please add me for (postgres|pg).*\.sql$ if possible please :)

This should actually be done in Gerrit (there is facility for that). Will have a look.

Suggestion from upstream was to add to rules.pl (could probably get away with doing it in All-Projects).

Anyone know Prolog?

We can use gerrit 'query' to get files changed by a patchset.

$ gerrit query 11899 --patch-sets --files --format=json > ~/JSON
+ ssh -p 29418 hashar@gerrit.wikimedia.org 'gerrit query 11899 --patch-sets --files --format=json'

Gives you a nice JSON structure to parse

"files":[
    {"file":"/COMMIT_MSG",
        "type":"ADDED"
    },
    {"file":"additions/php_file.php",
        "type":"ADDED"
    },
    {"file":"additions/sql_file.php",
        "type":"ADDED"
    }
]

The Gerrit trigger plugin does publish the change number as an environment variable (GERRIT_CHANGE_NUMBER). We could then write a script that would query Gerrit for the list of files that change involve (command above), then based on some logic the script could set a reviewer on the change (using gerrit set-reviewers).

Commands:

gerrit query $GERRIT_CHANGE_NUMBER --patch-sets --files --format=json

gerrit set-reviewers $GERRIT_CHANGE_NUMBER -add someone@example.org

(note, I am implicitly discarding implementing such a feature in Gerrit just because nobody knows prolog)

I added a very basic ant rule to detect if a SQL file got changed: https://gerrit.wikimedia.org/r/#/c/13169/

Need to wrap it in a Jenkins job called from the MediaWiki-GIT-Fetching job.

we need this for beta labs in order to update db tables in sync with updated code, ahead of production

we need this for beta labs in order to update db tables in sync with updated code, ahead of production

(In reply to comment #9)

we need this for beta labs in order to update db tables in sync with updated
code, ahead of production

Wouldn't it be better for that purpose to check in the deployment script if there were any SQL changes between $DEPLOYED_SHA1 and $SHA1_TO_DEPLOY instead of keeping books on changes to come?

One possibility would be to change update.php so it can be made to report any change made when doing the update. A Jenkins job could then install from master then update the code base to the change version and run update.php. Whenever update.php change something the Jenkins job could log / warn someone.

(In reply to comment #11)

One possibility would be to change update.php so it can be made to report any
change made when doing the update. A Jenkins job could then install from master
then update the code base to the change version and run update.php. Whenever
update.php change something the Jenkins job could log / warn someone.

That would be far too late, since it wouldn't notify the person until after it merged. Also it involves hacking up update.php just to support our workflow.

Got to be a better way.

I gather from the comments and the non-replies that not only need someone be added as a reviewer, but the merge itself should only happen if someone from beta/deployment green-lights it (+1 or +2).

So we need to:

• define this group,
• adapt rules.pl.

For the latter, we need:

• someone knowledgable in/willing to learn Prolog,
• rules.pl.

So let's start at the bottom: Where is it? Grepping over the operations/puppet and operations/gerrit didn't show anything obvious.

(In reply to comment #12)

(In reply to comment #11)

One possibility would be to change update.php so it can be made to report any
change made when doing the update. A Jenkins job could then install from master
then update the code base to the change version and run update.php. Whenever
update.php change something the Jenkins job could log / warn someone.

That would be far too late, since it wouldn't notify the person until after it
merged. Also it involves hacking up update.php just to support our workflow.

Got to be a better way.

Under a jenkins job, the patchset is not merged yet so that should be fine :-]

restoring bug summary. This is really about detecting a SQL change, not preventing the change to be merged.

(In reply to comment #0)

From wikitech-l by Rob Lanphier about DB changes
http://permalink.gmane.org/gmane.science.linguistics.wikipedia.technical/
60967

3. For anything that involves a schema change to the production dbs,

make sure Asher Feldman (<yum I really love spam>) is on the reviewer
list. He's already keeping an eye on this stuff the best he can, but
it's going to be easy for him to miss changes in extensions should
they happen.

I am pretty sure Jenkins could detect a change is being made on a .sql
file and then add a reviewer using the CLI tool.

(In reply to comment #2)

If you do this, please add me for (postgres|pg).*\.sql$ if possible please :)

I think this kind of conditional reviewer-adding is already working with [[mw:Git/Reviewers]]. I'm not sure this counts as solving the bug, but it works for me.

[Assignee was removed, hence also resetting ASSIGNED status]

This can be done in Gerrit by watching a project with a specially crafted search query. Simply head to https://gerrit.wikimedia.org/r/#/settings/projects then:

Project: mediawiki/core
Search: file:^.*\.sql$

Of courses some nasty SQL queries can be crafted from PHP files so there is nothing that can replace human review. Abandoning this bug.