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 DataGateway and PaNOSC Modes #256

Closed
3 tasks
MRichards99 opened this issue Sep 23, 2021 · 2 comments · Fixed by #285
Closed
3 tasks

Add DataGateway and PaNOSC Modes #256

MRichards99 opened this issue Sep 23, 2021 · 2 comments · Fixed by #285
Assignees
Labels
enhancement New feature or request expands-search-api Issues relating to the ExPaNDS Search API section of this repo

Comments

@MRichards99
Copy link
Collaborator

Description:
As DataGateway API and our implementation of the PaNOSC Search API will be developed in this repo, there needs to be a way of dealing with this. For the first release, I think it'll be best to add a configuration option for datagateway and panosc modes.

The datagateway mode will expose the current endpoints and the panosc mode will (upon implementing) expose the 11 endpoints for the search API. This config option will have to be made mandatory and can be done in config.py.

To do this, I need to add a config option (mode?) and use that option in main.py to branch off to the existing calls to api_start_utils.py and the new panosc function calls to setup the Flask app for that mode. To avoid api_start_utils.py from getting too long and confused, I might create a search_api_start_utils.py to have the endpoint definition and general app infrastructure setup there. At which point, it would probably make sense to rename the existing file to datagateway_api_start_utils.py.

In the future, we should look at supporting both at the same time (i.e. where a single instance can be run for both DataGateway API and PaNOSC Search API) by adding prefixes (perhaps something like /datagateway/ENDPOINT and /panosc/ENDPOINT) but that should come later.

Acceptance criteria:

  • Add a way to switch between DataGateway and PaNOSC modes, which should be configurable
  • Add mode config option to config.json.example
  • Add test in to cover the changing of modes
@MRichards99 MRichards99 added enhancement New feature or request expands-search-api Issues relating to the ExPaNDS Search API section of this repo labels Sep 23, 2021
@MRichards99
Copy link
Collaborator Author

[{"datagateway-api": "datagateway"}, {"panosc-api": "panosc"}]

[{datagateway-api": ""}]

no prefix (blank string) is all good as long as length of array is 1

Check that no prefix strings match across the list

{"datagateway": "datagateway-api", "panosc": "panosc-api"}

Could flip them round to avoid collisions and have multiple instances of the same API? Could require further reworking of config if you wanted to point two DG API at two different ICAT instances.

@MRichards99 MRichards99 self-assigned this Oct 12, 2021
@MRichards99
Copy link
Collaborator Author

MRichards99 commented Oct 20, 2021

My current suggested config format:

{
  "datagateway_api": {
    "extension": "/datagateway-api",
    "backend": "python_icat",
    "client_cache_size": 10,
    "client_pool_init_size": 3,
    "client_pool_max_size": 20,
    "db_url": "mysql+pymysql://icatdbuser:icatdbuserpw@localhost:3306/icatdb",
    "icat_url": "https://localhost.localdomain:8181",
    "icat_check_cert": false
  },
  "search_api": {
    "extension": "/search-api",
    "icat_url": "https://localhost.localdomain:8181",
    "icat_check_cert": false,
    "client_pool_init_size": 3
    "client_pool_max_size": 20
  },
  "flask_reloader": false,
  "log_level": "DEBUG",
  "log_location": "path/to/datagateway-api/logs.log",
  "debug_mode": true,
  "generate_swagger": true,
  "host": "127.0.0.1",
  "port": "5000",
  "test_user_credentials": {"username": "root", "password": "pw"},
  "test_mechanism": "simple"
}

This separates specific config for DataGateway API and the search API (in the relevant objects) and keeps the general config (e.g. flask_reloader) in the same base object as before. The extension key-value pair allows configuring of the end part of the URL. Not sure if extension is the correct term but you get the idea. If this config option is present, perhaps the extensions I've used above could be used as defaults? Depends how easy it is to implement that I guess.

Having DataGateway API and search API config in separate objects means having one API or both will be easy to implement. If neither are present in the config, then the only endpoint that should be present is the openapi one.

The behaviour where optional config isn't needed (i.e. no need for client config if using DB backend for DG API) should remain.

MRichards99 added a commit that referenced this issue Oct 25, 2021
- This format of config has not been implemented at time of commit
MRichards99 added a commit that referenced this issue Oct 25, 2021
- Since this repository will support DataGateway API and the Search API, there needs to be some changes in directory structure to split the different files up. Essentially this means adding `datagateway_api/` and `search_api/` in `common/` and `src/resources/`
- The imports will be fixed in a future commit
MRichards99 added a commit that referenced this issue Oct 25, 2021
- This format of config has not been implemented at time of commit
MRichards99 added a commit that referenced this issue Oct 25, 2021
- Since this repository will support DataGateway API and the Search API, there needs to be some changes in directory structure to split the different files up. Essentially this means adding `datagateway_api/` and `search_api/` in `common/` and `src/resources/`
- The imports will be fixed in a future commit
MRichards99 added a commit that referenced this issue Oct 26, 2021
- Reverting back so the tests pass until I re-implement the config
MRichards99 added a commit that referenced this issue Nov 3, 2021
- Since this repository will support DataGateway API and the Search API, there needs to be some changes in directory structure to split the different files up. Essentially this means adding `datagateway_api/` and `search_api/` in `common/` and `src/resources/`
- The imports will be fixed in a future commit
MRichards99 added a commit that referenced this issue Nov 3, 2021
- Reverting back so the tests pass until I re-implement the config
@MRichards99 MRichards99 assigned VKTB and unassigned MRichards99 Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 4, 2021
The test that was creating a temporary file was failing (even without
the config changes) due to Windows file permission issue. Though a unit
test should run in isolation and should not touch the file system at
all therefore I instead mocked the `builtins.open` to fake `with open`.
VKTB pushed a commit that referenced this issue Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 4, 2021
VKTB pushed a commit that referenced this issue Nov 9, 2021
BREAKING CHANGE: extend configuration to allow for different API modes
VKTB pushed a commit that referenced this issue Nov 9, 2021
VKTB pushed a commit that referenced this issue Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request expands-search-api Issues relating to the ExPaNDS Search API section of this repo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants