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

Data access api #720

Merged

Conversation

anamanica
Copy link
Contributor

Change Summary

Overview

Changing the Data Access API documentation to include new features, note important information, and overall just make things more clear for users.

New Dependencies

None

New Files

None

Deleted Files

None

Updated Files

  • index.rst
    • Added several notes into the end of the upload, download, and intro section of the docs.
  • openapi.yaml
    • Added new query options to the query section of the docs

Testing

None

closes #648

@anamanica anamanica changed the title [WIP] Data access api Data access api Jul 26, 2024
@anamanica anamanica marked this pull request as ready for review July 26, 2024 21:00
Copy link
Contributor

@daralynnrhode daralynnrhode left a comment

Choose a reason for hiding this comment

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

Besides the one spot, It all looks good to me!

@@ -64,6 +78,8 @@ The API can be accessed from the following URL: https://api.dev.imap-mission.com
:group:
:include: /query

*Note:* ``filename``, ``N days ago`` *, and* ``today`` *are all [WIP].*
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for clarity sake it may be better to note these as WIP in their descriptions (or tag it onto the name ) in openapi.yml.

Merge branch 'dev' of github.com:IMAP-Science-Operations-Center/imap_processing into data-access-api
Copy link
Collaborator

@bourque bourque left a comment

Choose a reason for hiding this comment

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

Thanks for making these updates! I have a few small suggestions, otherwise this looks good!

This can be in a future PR, but I would like to see these docs contain all of the info that is currently provided in the imap-data-access repo's README file. I created an issue for this: IMAP-Science-Operations-Center/imap-data-access#75. In that way, all of the documentation for the IMAP Data Access API will be in one place, instead of spread between the two.

Comment on lines 14 to 17
The API can be accessed from the following URL [WIP]: https://api.dev.imap-mission.com

*Note: Several sections and links begin with* [WIP]. *As development on the API is ongoing, this indicates
that the full implementation of the functionality is yet to be completed.*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
The API can be accessed from the following URL [WIP]: https://api.dev.imap-mission.com
*Note: Several sections and links begin with* [WIP]. *As development on the API is ongoing, this indicates
that the full implementation of the functionality is yet to be completed.*
*Note: Several sections and links begin with* [WIP]. *As development on the API is ongoing, this indicates
that the full implementation of the functionality is yet to be completed.*
The API can be accessed from the following URL [WIP]: https://api.dev.imap-mission.com

I like that you added this! I would prefer that the note go before the first use of [WIP] so the reader is aware of what it means before they first encounter it, but its up to you!

@@ -143,7 +144,8 @@ paths:
type: string
- in: query
name: version
description: The version of data product in the format ``vXX-YY`` (e.g. ``v01-01``).
description: The version of data product in the format ``vXX-YY`` (e.g. ``v01-01``). You can also choose to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
description: The version of data product in the format ``vXX-YY`` (e.g. ``v01-01``). You can also choose to
description: The version of data product in the format ``vNNN`` (e.g. ``v001``). You can also choose to

This documentation has become outdated and still references the old version number format. Could you update this?

Comment on lines 158 to 175
- in: query
name: filename
description: WIP -- The full filename of the desired file (e.g. ``imap_mag_l1a_burst_20240105_20240105_v01-01.pkts``).
required: false
schema:
type: string
- in: query
name: N_days_ago
description: WIP -- Returns all files acquired in the last 'N' days (with 'N' representing a numeric value).
required: false
schema:
type: string
- in: query
name: today
description: WIP -- Returns all files acquired in the last 24 hours.
required: false
schema:
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to add these in as we implement these features so we get in the habit of updating the docs in the same PRs that implement said features. Also, that would avoid a situation in which the docs say 'we are going to add this feature' but we don't actually get around to implementing it for several months, which could be a bad look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree with you!

Merge branch 'dev' of github.com:IMAP-Science-Operations-Center/imap_processing into data-access-api
@anamanica anamanica requested a review from bourque August 7, 2024 15:45
Copy link
Collaborator

@bourque bourque left a comment

Choose a reason for hiding this comment

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

Thanks for updating, looks good!

@anamanica anamanica merged commit 8222c1b into IMAP-Science-Operations-Center:dev Aug 7, 2024
17 checks passed
@anamanica anamanica deleted the data-access-api branch August 7, 2024 21:44
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.

General Documentation Update for Data Access API
3 participants