Fix swagger docs at non-root paths #408

[louise-davies]

This PR will close #408

Description

Add a url_prefix config item which tells the app where it's expected to be mounted. I've added some code to take url_prefix into consideration in development, but otherwise we'd be expecting this to be done via a WSGI server e.g. Apache mod_wsgi

This then fixes the swagger UI as the UI files & openapi.json have the correct URLs. Every other URL in the app already worked fine with aliases as they're handled as part of WSGI spec

Testing Instructions

Add a set up instructions describing how the reviewer should test the code

• Review code
• Check GitHub Actions build
• If icatdb Generator Script Consistency Test CI job fails, is this because of a deliberate change made to the script to change generated data (which isn't actually a problem) or is here an underlying issue with the changes made?
• Review changes to test coverage
• Does this change mean a new patch, minor or major version should be made? If so, does one of the commit messages feature fix:, feat: or BREAKING CHANGE: so a release is automatically made via GitHub Actions upon merge?
• Run API locally with url_prefix option and set one of dg-api or search-api's aliases to "/" - test that the swagger UI is visible for both APIs
• Ideally also test with Apache mod_wsgi - or I can do a call with you and show you on my machine :)

Agile Board Tracking

Closes #408

[codecov]

Codecov Report

Patch coverage: 83.33% and project coverage change: -0.05 ⚠️

Comparison is base (9921b53) 96.84% compared to head (0ed0078) 96.80%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #409      +/-   ##
==========================================
- Coverage   96.84%   96.80%   -0.05%     
==========================================
  Files          40       40              
  Lines        3363     3375      +12     
  Branches      316      317       +1     
==========================================
+ Hits         3257     3267      +10     
- Misses         78       80       +2     
  Partials       28       28              

Impacted Files	Coverage Δ
datagateway_api/src/common/config.py	97.84% <80.00%> (-2.16%)	⬇️
...tagateway_api/src/swagger/apispec_flask_restful.py	54.23% <100.00%> (+1.60%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[VKTB]

Sorry, I am late seeing this PR but I just noticed that it is introducing a new config value. From a versioning perspective, I think that this should be a breaking change (major version bump) rather than a fix (patch version bump).

(venv) [datagateway-api]$ semantic-release version
Creating new version
Current version: 7.1.0, Current release version: 7.1.0
Bumping with a patch version to 7.1.1

[MRichards99]

@VKTB ah yes, I missed that (it looks like the Semantic Release workflow thing doesn't run any more either). I think rewording 26a901e758c1f45dd0b747fae049e7a0d32c9b89 so it causes a bump to 8.0.0 would be the best thing to do? I've made the change locally so let me know if you agree and I can just force-push

[VKTB]

I think rewording 26a901e758c1f45dd0b747fae049e7a0d32c9b89 so it causes a bump to 8.0.0 would be the best thing to do? I've made the change locally so let me know if you agree and I can just force-push

@MRichards99 Yes that should do the job. While at it, could you also please merge main into this branch as it is behind?

it looks like the Semantic Release workflow thing doesn't run any more either

From its repo README, it Looks like it is no longer maintained and there was a security incident. It does mention some alternatives but we need to make sure that they are secure before we use them.

[MRichards99]

@VKTB It looks like https://github.com/Ezard/semantic-prs would a good alternative

[VKTB]

All good now, thanks for sorting this out @MRichards99!

(venv) [datagateway-api]$ semantic-release print-version
8.0.0

[RKrahl]

Dear all, many thanks for working on this!

I tried it out, it clearly shows progress, but we are not there, I'm afraid.

Test setup

I deployed datagateway_api in Apache with mod_wsgi. The relevant bits of the config was like:

httpd.conf:

<VirtualHost _default_:443>

        ServerName data.example.org:443

        WSGIDaemonProcess datagateway \
                display-name=%{GROUP} user=wwwrun group=www threads=3
        WSGIScriptAlias /api /usr/lib/python3.6/site-packages/datagateway_api/wsgi.py \
                process-group=datapub application-group=%{GLOBAL}

</VirtualHost>

config.yaml:

datagateway_api:
  extension: "/datagateway"
  # ...
search_api:
  extension: "/panosc"
  # ...
url_prefix: "/api"

Observations

As expected, I could reach the Swagger UI for the datagateway and for search API at https://data.example.org/api/datagateway and https://data.example.org/api/panosc respectively. It looked good at first glance.

But the endpoints in the swagger UI were listed as:

Dataset
GET /panosc/Datasets
GET /panosc/Datasets/{pid}
...

and the calls after hitting Try it out -> Execute were made to https://data.example.org/panosc/Datasets/..., which of course yields a 404. The corresponding calls to https://data.example.org/api/panosc/Datasets/... seem to work. So the Swagger UI was still kind of assuming the application to be deployed at root, dropping the url_prefix in the path of all internal URLs.

[louise-davies]

@RKrahl sorry about that - I overlooked that the URLs in the swagger UI were wrong. I've fixed it so it now displays the correct URLs - respecting the url_prefix - which can then be used to query the API in the swagger UI sucessfully.

[RKrahl]

Great! I'll have a look.

[RKrahl]

Tested, works, looks good!