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

Chore/setup update api v2 #138

Merged
merged 14 commits into from
Nov 14, 2024
Merged

Chore/setup update api v2 #138

merged 14 commits into from
Nov 14, 2024

Conversation

thepsalmist
Copy link
Contributor

@thepsalmist thepsalmist commented Oct 2, 2024

Description

The PR introduces build fixes and dependency upgrades & patches for implementation of authentication for key endpoints prone to bot crawling. The affected endpoints/views in API v1 are data, now and node. The API endpoints in v2 all use Token and Session authentication.

Push sensors data endpoint still remains on v1/push-sensor-data. Note: This would require firmware upgrade

API v1 endpoint API v2 endpoint
/api/v1/data. /api/v2/data.
/api/v1/node. /api/v1/node.
/api/v1/now. /api/v2/now.
/api/v1/statistics /api/v1/statistics
api/v1/sensor /api/v2/sensors

Fixes issues #134, #135, #136

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation

@thepsalmist thepsalmist added bug Something isn't working chore labels Oct 2, 2024
@thepsalmist thepsalmist self-assigned this Oct 2, 2024
@gideonmaina
Copy link
Collaborator

@thepsalmist all v2 endpoints passed but a confusion may arise with v2/nodes vs v1/node. The former registers a node to the database via a POST request while the latter lists nodes owned by an authenticated user. A more descriptive name for the endpoints would solve this. E.g. /add-node or /register-node and /my-nodes

Check out API endpoints list we have on V2 and V1

endpoint v2 (staging) v1 (production)
1./data ok ok
2. /cities ok missing
3. /now ok ok
4. /locations ok missing
5. /sensors ok missing
6. /sensor-types ok missing
7. /statistics ok ok
8. /nodes ok missing
9. /node missing ok
10. /user missing ok
11. /meta ok missing
12. /sensor missing ok

The /sensor and /user are not useful in my opinion therefore they can be excluded in v2.

@thepsalmist
Copy link
Contributor Author

@thepsalmist all v2 endpoints passed but a confusion may arise with v2/nodes vs v1/node. The former registers a node to the database via a POST request while the latter lists nodes owned by an authenticated user. A more descriptive name for the endpoints would solve this. E.g. /add-node or /register-node and /my-nodes

Check out API endpoints list we have on V2 and V1

endpoint v2 (staging) v1 (production)
1./data ok ok
2. /cities ok missing
3. /now ok ok
4. /locations ok missing
5. /sensors ok missing
6. /sensor-types ok missing
7. /statistics ok ok
8. /nodes ok missing
9. /node missing ok
10. /user missing ok
11. /meta ok missing
12. /sensor missing ok
The /sensor and /user are not useful in my opinion therefore they can be excluded in v2.

Thanks @gideonmaina updating as per above

@gideonmaina
Copy link
Collaborator

🚨🚨 /v1/push-sensor-data does not allow POST requests @thepsalmist. Also, push_sensor_data_urls router is appended to the endpoint as /v1/push-sensor-data/push-sensor-data


Screenshot 2024-10-07 at 16 59 03
Screenshot 2024-10-07 at 16 59 20

@thepsalmist
Copy link
Contributor Author

@thepsalmist all v2 endpoints passed but a confusion may arise with v2/nodes vs v1/node. The former registers a node to the database via a POST request while the latter lists nodes owned by an authenticated user. A more descriptive name for the endpoints would solve this. E.g. /add-node or /register-node and /my-nodes
Check out API endpoints list we have on V2 and V1
endpoint v2 (staging) v1 (production)
1./data ok ok
2. /cities ok missing
3. /now ok ok
4. /locations ok missing
5. /sensors ok missing
6. /sensor-types ok missing
7. /statistics ok ok
8. /nodes ok missing
9. /node missing ok
10. /user missing ok
11. /meta ok missing
12. /sensor missing ok
The /sensor and /user are not useful in my opinion therefore they can be excluded in v2.

Thanks @gideonmaina updating as per above

The above issues have been resolved

@thepsalmist
Copy link
Contributor Author

🚨🚨 /v1/push-sensor-data does not allow POST requests @thepsalmist. Also, push_sensor_data_urls router is appended to the endpoint as /v1/push-sensor-data/push-sensor-data

Screenshot 2024-10-07 at 16 59 03 Screenshot 2024-10-07 at 16 59 20

Resolved

@gideonmaina
Copy link
Collaborator

gideonmaina commented Oct 23, 2024

Observations for v2 endpoints:

  1. /nodes/list-nodes: this can go just to /nodes
  2. sensors/{sensor_id}: still missing. We can implement this later alongside other filtering requests like /nodes/{node_id}

@gideonmaina
Copy link
Collaborator

@thepsalmist There is also this v1/sensor/ endpoint that does not make sense. On v2 this not need as we will have /sensors to list all sensors and filter a sensor via /sensors/{sensor_id}

@gideonmaina
Copy link
Collaborator

@thepsalmist the endpoints called from the frontend currently are:

v1

  1. https://api.sensors.africa/v1/sensors/{sensor_id}/
  2. https://api.sensors.africa/v1/filter?city=&country=&type=
  3. https://api.sensors.africa/v1/data/
  4. https://api.sensors.africa/v1/now/
  5. http://api.sensors.africa/v1/sensor/%7BsensorID%7D/

v2

  1. v2/cities
  2. v2/data/air?city={slug}&
  3. v2/nodes
  4. v2/data.json
  5. v2/data.1h.json
  6. v2/data.24h.json
  7. v2/data.dust.min.json
  8. v2/data.temp.min.json

and this filter https://api.sensors.africa/v2/data/air/?city=${city}&from=${fromDate}&interval=day&value_type=P2

Copy link
Collaborator

@gideonmaina gideonmaina left a comment

Choose a reason for hiding this comment

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

All v2 endpoints are working as expected 🙌🏼.

These observations/ recommendations can be a new issue.

Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is Django v >=1.10

sensorsafrica/api/v1/router.py Show resolved Hide resolved
sensorsafrica/urls.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@gideonmaina
Copy link
Collaborator

Hey @thepsalmist what is holding us back?

@thepsalmist thepsalmist merged commit 607b559 into master Nov 14, 2024
2 checks passed
@thepsalmist thepsalmist deleted the chore/setup-update-api-v2 branch November 14, 2024 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants