Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add endpoint for listing all WebAPI routes #6070

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

perlpunk
Copy link
Contributor

@perlpunk perlpunk commented Nov 26, 2024

% openqa-cli api routes | jq
{                 
  "routes": [                     
    {             
      "methods": [
        "GET"                                     
      ],                                 
      "path": "/"
    },                                              
...
    {
      "methods": [
        "DELETE",                 
        "GET"     
      ],          
      "path": "/api/v1/assets/<id:num>"           
    },                                   

I find it cumbersome to find out routes currently. The webpage 404 page, e.g. https://openqa.opensuse.org/foo , is very long and I lose the overview under which top level route I am.

This could also be useful later for creating dynamic completion, e.g. openqa-cli api j<TAB>

@perlpunk perlpunk force-pushed the routes branch 3 times, most recently from b38f27d to 6fdf5f6 Compare November 26, 2024 18:33
Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea and it might indeed be interesting for dynamic completion.

@perlpunk
Copy link
Contributor Author

No codecov result. I'll force push. I would expect that the module appears as uncovered, as I didn't add any tests so far

@Martchus
Copy link
Contributor

Still no results. Recent webhook deliveries look good but there's probably some problem on the Codecov side right now.

Copy link

codecov bot commented Nov 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.98%. Comparing base (db480fd) to head (c269ac2).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6070   +/-   ##
=======================================
  Coverage   98.98%   98.98%           
=======================================
  Files         395      396    +1     
  Lines       39508    39527   +19     
=======================================
+ Hits        39108    39127   +19     
  Misses        400      400           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@okurz
Copy link
Member

okurz commented Nov 28, 2024

… and now there is the report :)

my $pattern = $route->pattern->unparsed || '';
my @methods;
if ($route->can('methods')) {
@methods = @{($route->methods) || []};
Copy link
Contributor

@b10n1k b10n1k Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the empty list should never be empty. I am not sure how you can handle this but it feels wrong, no? or maybe I do not understand something here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. An empty list is empty.
methods might return undef, in which case @{ somehing undef } will error, so @{ somehing undef || []} is needed.
btw, I copied this code from https://github.com/os-autoinst/openQA/blob/master/templates/webapi/not_found.html.ep#L26

@perlpunk
Copy link
Contributor Author

perlpunk commented Dec 2, 2024

I added a test and simplified the _walk method a bit

@mergify mergify bot merged commit 010f59e into os-autoinst:master Dec 3, 2024
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants