load_templates: Create groups that do not exist #2770
Conversation
kalikiana
changed the title
kalikiana
force-pushed
the
load_templates_create_group
branch
3 times, most recently
from
54b301d
to
a7a931e
Compare
|
There was a The server seemed to create an OK response with no JSON. @andrii-suse pointed me towards MOJO_MAX_LINE_SIZE which seemed to work around the problem until finally with the assistance of @perlpunk it was revealed that the I'm going to propose another PR to update other code paths with the more reliable API uses. This branch is now back to the original fix. |
There was a problem hiding this comment.
I would still propose to use a local $url variable, instead of modifying the global one.
There can be other manipulations to that variable that are not wanted.
So wherever the $url variable is used, replace that with:
my $copy = $url->clone;
# work with $copyMove from query to form => syntax and use is_success in new code. Fixes: https://progress.opensuse.org/issues/60118
kalikiana
force-pushed
the
load_templates_create_group
branch
from
a7a931e
to
a291931
Compare
| # Create the group first | ||
| my $job_groups_url = $url->clone->path($options{'apibase'} . '/job_groups'); | ||
| my $res = $client->post($job_groups_url, form => {name => $entry->{group_name}})->res; | ||
| print_error $res unless $res->is_success; |
There was a problem hiding this comment.
Do we intentionally continue on error here?
There was a problem hiding this comment.
We don't anymore. In another branch I made print_error fatal and I'm changing other call sites bit by bit.
There was a problem hiding this comment.
See #2776 for the follow-up branch
| @@ -58,22 +58,16 @@ is $ret, 255, 'error because of insufficient permissions'; | |||
| $apikey = 'ARTHURKEY01'; | |||
| $apisecret = 'EXCALIBUR'; | |||
| $args = "--host $host --apikey $apikey --apisecret $apisecret $filename"; | |||
| combined_like sub { $ret = run_once($args); }, qr/Job group.+not found/, 'Invalid job group'; | |||
There was a problem hiding this comment.
It looks we removed scenario when 'Job group not found' is returned. (just observation)
There was a problem hiding this comment.
Right. This is because we do handle the case of creating the group now.
This is a follow-up to os-autoinst#2770. Re-using the URL can lead to problems which are difficult to diagnose. Instead we should always use the form style which works for get, put and post.
Codecov Report
@@ Coverage Diff @@
## master #2770 +/- ##
=========================================
- Coverage 92.21% 92.2% -0.01%
=========================================
Files 190 190
Lines 11821 11821
=========================================
- Hits 10901 10900 -1
- Misses 920 921 +1
Continue to review full report at Codecov.
|
The new code uses
form =>syntax to post without making use of the query and alsois_successto bail out.Fixes: poo#60118