-
Notifications
You must be signed in to change notification settings - Fork 6
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
Upload task orthophoto to OpenAerialMap (OAM) #346
base: develop
Are you sure you want to change the base?
Conversation
… the project requires regulators approval
for more information, see https://pre-commit.ci
…ne-tm into feat/upload-orthophoto-to-oam
Uploads an orthophoto (TIFF) file to OpenAerialMap (OAM). | ||
|
||
Args: | ||
project_id: The UUID of the project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only really need to add docstrings with Args
and Returns
on crud functions. For routes, this is automatically documented using type hints and OpenAPI spec 👌
Plus docstrings are useful for devs to consume functions, but the route is the top level of the logic and nobody is importing and using the routes elsewhere
@@ -86,3 +89,43 @@ async def new_event( | |||
user_data, | |||
background_tasks, | |||
) | |||
|
|||
|
|||
@router.post("/upload/{project_id}/{task_id}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably isn't a comment specific to this PR, but the project as a whole.
The endpoint structure for FMTM doesn't necessarily conform to the REST spec / best practices, but we are working on it & hopefully that's changing over time 🙏
So typically it would be set up like this:
If the task id is a unique identifier on it's own.
/projects/{project_id}
/projects/{project_id}/do-something
/tasks/{task_id}
/tasks/{task_id}/do-something
If task ids are only unique in relation to the project they are within:
/projects/{project_id}
/projects/{project_id}/tasks/{task_id}
/projects/{project_id}/tasks/{task_id}/do-something
So for this endpoint, as task_id is a globally unique UUID, it would make sense to create:
/tasks/{task_id}/upload-ortho
or something similar.
Just for info going forward! I'm not saying things have to change immediately, or even at all, but it could be a nice to have into the future 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
I think we can add some tags into OpenAerialMap. We can add some default tags like And also instead of uploading the imagery from our account, it might be better to ask the users for their own token . What do you say? @ivangayton @spwoodcock |
Ideally, it should be the project creator who gets the credit for the OAM upload |
Definitely! Add #drone-tm and something like #{domain_name}-{project-id} as a reference to start. Also agree with Ivan to attribute the project manager who clicks the upload button. Can you use the oauth flow for OAM to log in? Then I assume you can use the oauth token from OAM to upload as a specific user. |
Description:
This PR introduces a new endpoint for uploading an orthophoto (TIFF file) to OpenAerialMap (OAM).