| Message ID | f7th8daqrfy.fsf@dhcp-25.97.bos.redhat.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | [ovs-dev,RFC] OVS robot discussion - check API integration | expand |
On Mon, Feb 11, 2019 at 10:24:01AM -0500, Aaron Conole wrote: > I'm following up on a discussion where we briefly talked about adding > a 'check' to OVS patch series during the OVS Conference. 'Check' is > what the patchwork project provides to show that certain tests have been > performed. It is made up of a name, a status, and a URL. Some of the > projects in the ozlabs patchwork instance are using the check feature > already, so there's some examples to reference. > > My idea is to push some data into travis / cirrus that can be used to > insert a check against a patch series. This would allow committers to > look at the status page on patchwork and see at a glance the links to > the travis, and if a robot email was sent. The approach seems reasonable. You mention that there are existing examples. How do they do it?
11/02/2019 18:03, Ben Pfaff: > On Mon, Feb 11, 2019 at 10:24:01AM -0500, Aaron Conole wrote: > > I'm following up on a discussion where we briefly talked about adding > > a 'check' to OVS patch series during the OVS Conference. 'Check' is > > what the patchwork project provides to show that certain tests have been > > performed. It is made up of a name, a status, and a URL. Some of the > > projects in the ozlabs patchwork instance are using the check feature > > already, so there's some examples to reference. > > > > My idea is to push some data into travis / cirrus that can be used to > > insert a check against a patch series. This would allow committers to > > look at the status page on patchwork and see at a glance the links to > > the travis, and if a robot email was sent. > > The approach seems reasonable. > > You mention that there are existing examples. How do they do it? In DPDK project, we are sending the results to a specific mailing-list with this script: http://git.dpdk.org/tools/dpdk-ci/tree/tools/send-patch-report.sh It is possible to filter some reports by using mail filtering. Then it is parsed on the server by the following script which updates patchwork data using server-specific credentials: http://git.dpdk.org/tools/dpdk-ci/tree/tools/update-pw.sh Note that we improved patchwork to highlight CI results with colors: https://patches.dpdk.org
On 11.02.2019 18:24, Aaron Conole wrote: > I'm following up on a discussion where we briefly talked about adding > a 'check' to OVS patch series during the OVS Conference. 'Check' is > what the patchwork project provides to show that certain tests have been > performed. It is made up of a name, a status, and a URL. Some of the > projects in the ozlabs patchwork instance are using the check feature > already, so there's some examples to reference. > > My idea is to push some data into travis / cirrus that can be used to > insert a check against a patch series. This would allow committers to > look at the status page on patchwork and see at a glance the links to > the travis, and if a robot email was sent. > > The naivest way I can think of getting these results published is by > modifying ${OS}-build.sh like: > > --- > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh > index 0a2091061..38359c86f 100755 > --- a/.travis/linux-build.sh > +++ b/.travis/linux-build.sh > @@ -8,6 +8,47 @@ SPARSE_FLAGS="" > EXTRA_OPTS="" > TARGET="x86_64-native-linuxapp-gcc" > > +function start_patchwork_check() > +{ > + if [ ! -e ".git/config" ]; then > + return; > + fi > + > + if ! git branch | grep \\* | grep series_ >/dev/null; then > + return; > + fi > + > + curl -s $PW_API_URL/$(SERIES_ID) ...submit check start ... > +} > + > +function bad_check_result() > +{ > + if [ ! -e ".git/config" ]; then > + return; > + fi > + > + if ! git branch | grep \\* | grep series_ >/dev/null; then > + return; > + fi > + > + curl -s $PW_API_URL/$(SERIES_ID) ...set check to bad... > +} > + > +function check_success() > +{ > + if [ ! -e ".git/config" ]; then > + return; > + fi > + > + if ! git branch | grep \\* | grep series_ >/dev/null; then > + return; > + fi > + > + curl -s $PW_API_URL/$(SERIES_ID) ...set check to success... > +} > + > +trap '[ "$?" -eq 0 ] || bad_check_result' INT ERR TERM > + > function install_kernel() > { > if [[ "$1" =~ ^4.* ]]; then > --- > > I think this could work. We can use the travis "encrypted info" data to > push the credentials but that probably needs to be encrypted for the > robot key. It does put some kind of pollution in the travis / cirrus > builds (because 'series_*' appears nowhere in the official repository). > Maybe that isn't acceptable. > > I am planning to follow the guidebook at > https://docs.travis-ci.com/user/environment-variables/#defining-variables-in-repository-settings > and store the credentials variables for just the OVS robot repo. > > Maybe there's another opinion. Probably there's a better way of > sending the data around, sharing credentials, or passing requests so > that other projects can also integrate. Thoughts, opinions, comments? I don't know if it could be an issue, but I just wanted to warn about 'set -x' that we have now in a Travis scripts. You need to be careful with exported credentials. And we, actually, already have special ovs-build mail-list. It could be used if you'll decide to choose DPDK way of working with patchwork. Best regards, Ilya Maximets.
Ilya Maximets <i.maximets@samsung.com> writes: > On 11.02.2019 18:24, Aaron Conole wrote: >> I'm following up on a discussion where we briefly talked about adding >> a 'check' to OVS patch series during the OVS Conference. 'Check' is >> what the patchwork project provides to show that certain tests have been >> performed. It is made up of a name, a status, and a URL. Some of the >> projects in the ozlabs patchwork instance are using the check feature >> already, so there's some examples to reference. >> >> My idea is to push some data into travis / cirrus that can be used to >> insert a check against a patch series. This would allow committers to >> look at the status page on patchwork and see at a glance the links to >> the travis, and if a robot email was sent. >> >> The naivest way I can think of getting these results published is by >> modifying ${OS}-build.sh like: >> >> --- >> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh >> index 0a2091061..38359c86f 100755 >> --- a/.travis/linux-build.sh >> +++ b/.travis/linux-build.sh >> @@ -8,6 +8,47 @@ SPARSE_FLAGS="" >> EXTRA_OPTS="" >> TARGET="x86_64-native-linuxapp-gcc" >> >> +function start_patchwork_check() >> +{ >> + if [ ! -e ".git/config" ]; then >> + return; >> + fi >> + >> + if ! git branch | grep \\* | grep series_ >/dev/null; then >> + return; >> + fi >> + >> + curl -s $PW_API_URL/$(SERIES_ID) ...submit check start ... >> +} >> + >> +function bad_check_result() >> +{ >> + if [ ! -e ".git/config" ]; then >> + return; >> + fi >> + >> + if ! git branch | grep \\* | grep series_ >/dev/null; then >> + return; >> + fi >> + >> + curl -s $PW_API_URL/$(SERIES_ID) ...set check to bad... >> +} >> + >> +function check_success() >> +{ >> + if [ ! -e ".git/config" ]; then >> + return; >> + fi >> + >> + if ! git branch | grep \\* | grep series_ >/dev/null; then >> + return; >> + fi >> + >> + curl -s $PW_API_URL/$(SERIES_ID) ...set check to success... >> +} >> + >> +trap '[ "$?" -eq 0 ] || bad_check_result' INT ERR TERM >> + >> function install_kernel() >> { >> if [[ "$1" =~ ^4.* ]]; then >> --- >> >> I think this could work. We can use the travis "encrypted info" data to >> push the credentials but that probably needs to be encrypted for the >> robot key. It does put some kind of pollution in the travis / cirrus >> builds (because 'series_*' appears nowhere in the official repository). >> Maybe that isn't acceptable. >> >> I am planning to follow the guidebook at >> https://docs.travis-ci.com/user/environment-variables/#defining-variables-in-repository-settings >> and store the credentials variables for just the OVS robot repo. >> >> Maybe there's another opinion. Probably there's a better way of >> sending the data around, sharing credentials, or passing requests so >> that other projects can also integrate. Thoughts, opinions, comments? > > I don't know if it could be an issue, but I just wanted to warn about > 'set -x' that we have now in a Travis scripts. You need to be careful > with exported credentials. Keen observation! > And we, actually, already have special ovs-build mail-list. It could > be used if you'll decide to choose DPDK way of working with patchwork. It looks intriguing. I like the idea that anyone could then contribute 'check results' to the patchwork project. But, it seems like it would require some additional infrastructure. We would need a script that will periodically run against the list, or possibly an alias that is subscribed to the list that is just an alias to /bin/process_ovs_build_mails.sh that can update the check interface. > Best regards, Ilya Maximets.
13/02/2019 17:56, Aaron Conole: > Ilya Maximets <i.maximets@samsung.com> writes: > > And we, actually, already have special ovs-build mail-list. It could > > be used if you'll decide to choose DPDK way of working with patchwork. > > It looks intriguing. I like the idea that anyone could then contribute > 'check results' to the patchwork project. But, it seems like it would > require some additional infrastructure. We would need a script that > will periodically run against the list, or possibly an alias that is > subscribed to the list that is just an alias to > /bin/process_ovs_build_mails.sh that can update the check interface. The best is to have a hook in the mail archiving system, so you can reference the report in the mail archives as soon as it is published.
Thomas Monjalon <thomas@monjalon.net> writes: > 13/02/2019 17:56, Aaron Conole: >> Ilya Maximets <i.maximets@samsung.com> writes: >> > And we, actually, already have special ovs-build mail-list. It could >> > be used if you'll decide to choose DPDK way of working with patchwork. >> >> It looks intriguing. I like the idea that anyone could then contribute >> 'check results' to the patchwork project. But, it seems like it would >> require some additional infrastructure. We would need a script that >> will periodically run against the list, or possibly an alias that is >> subscribed to the list that is just an alias to >> /bin/process_ovs_build_mails.sh that can update the check interface. > > The best is to have a hook in the mail archiving system, > so you can reference the report in the mail archives > as soon as it is published. I'm not sure whether anyone can run such a script on the ovs mail server. Regardless, if there is something subscribed to the list, we can probably scan the list archives for the appropriate url and update with that link.
> On Feb 25, 2019, at 10:42 AM, Aaron Conole <aconole@redhat.com> wrote: > > Thomas Monjalon <thomas@monjalon.net> writes: > >> The best is to have a hook in the mail archiving system, >> so you can reference the report in the mail archives >> as soon as it is published. > > I'm not sure whether anyone can run such a script on the ovs mail > server. Regardless, if there is something subscribed to the list, we > can probably scan the list archives for the appropriate url and update > with that link. Right. Linux Foundation runs our mail server, so this would probably be a hard sell. :-) If we could run something externally, I imagine that will be easier. --Justin
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index 0a2091061..38359c86f 100755 --- a/.travis/linux-build.sh +++ b/.travis/linux-build.sh @@ -8,6 +8,47 @@ SPARSE_FLAGS="" EXTRA_OPTS="" TARGET="x86_64-native-linuxapp-gcc" +function start_patchwork_check() +{ + if [ ! -e ".git/config" ]; then + return; + fi + + if ! git branch | grep \\* | grep series_ >/dev/null; then + return; + fi + + curl -s $PW_API_URL/$(SERIES_ID) ...submit check start ... +} + +function bad_check_result() +{ + if [ ! -e ".git/config" ]; then + return; + fi + + if ! git branch | grep \\* | grep series_ >/dev/null; then + return; + fi + + curl -s $PW_API_URL/$(SERIES_ID) ...set check to bad... +} + +function check_success() +{ + if [ ! -e ".git/config" ]; then + return; + fi + + if ! git branch | grep \\* | grep series_ >/dev/null; then + return; + fi + + curl -s $PW_API_URL/$(SERIES_ID) ...set check to success... +} + +trap '[ "$?" -eq 0 ] || bad_check_result' INT ERR TERM + function install_kernel() { if [[ "$1" =~ ^4.* ]]; then
I'm following up on a discussion where we briefly talked about adding a 'check' to OVS patch series during the OVS Conference. 'Check' is what the patchwork project provides to show that certain tests have been performed. It is made up of a name, a status, and a URL. Some of the projects in the ozlabs patchwork instance are using the check feature already, so there's some examples to reference. My idea is to push some data into travis / cirrus that can be used to insert a check against a patch series. This would allow committers to look at the status page on patchwork and see at a glance the links to the travis, and if a robot email was sent. The naivest way I can think of getting these results published is by modifying ${OS}-build.sh like: --- --- I think this could work. We can use the travis "encrypted info" data to push the credentials but that probably needs to be encrypted for the robot key. It does put some kind of pollution in the travis / cirrus builds (because 'series_*' appears nowhere in the official repository). Maybe that isn't acceptable. I am planning to follow the guidebook at https://docs.travis-ci.com/user/environment-variables/#defining-variables-in-repository-settings and store the credentials variables for just the OVS robot repo. Maybe there's another opinion. Probably there's a better way of sending the data around, sharing credentials, or passing requests so that other projects can also integrate. Thoughts, opinions, comments?