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

feat: add endpoint to upload task orthophoto to OpenAerialMap (OAM) #346

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Pradip-p
Copy link
Collaborator

@Pradip-p Pradip-p commented Nov 20, 2024

Description:

This PR introduces a new endpoint for uploading an orthophoto (TIFF file) to OpenAerialMap (OAM).

  • Implemented a POST endpoint at /upload/{project_id}/{task_id} for uploading orthophotos (TIFF files) to OpenAerialMap.
  • Added functionality to generate S3 URL for the orthophoto and initiate the upload to OAM using the OAM API token.
  • Returns OAM upload ID for tracking the upload status.

@Pradip-p Pradip-p requested a review from nrjadkry November 20, 2024 11:46
@Pradip-p Pradip-p self-assigned this Nov 20, 2024
@github-actions github-actions bot added enhancement New feature or request backend Related to backend code frontend labels Nov 20, 2024
@nrjadkry nrjadkry requested a review from spwoodcock November 21, 2024 10:45
@nrjadkry nrjadkry added the OAM label Nov 21, 2024
Uploads an orthophoto (TIFF) file to OpenAerialMap (OAM).

Args:
project_id: The UUID of the project.
Copy link
Member

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}")
Copy link
Member

@spwoodcock spwoodcock Nov 21, 2024

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 😄

Copy link
Member

@spwoodcock spwoodcock left a comment

Choose a reason for hiding this comment

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

Looking good!

@nrjadkry
Copy link
Member

nrjadkry commented Nov 26, 2024

I think we can add some tags into OpenAerialMap. We can add some default tags like #drone-tm and ask the users for additional tags when uploading to OAM.

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

@ivangayton
Copy link
Collaborator

Ideally, it should be the project creator who gets the credit for the OAM upload

@spwoodcock
Copy link
Member

I think we can add some tags into OpenAerialMap. We can add some default tags like #drone-tm and ask the users for additional tags when uploading to OAM.

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

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.
Or perhaps they have an endpoint to generate API keys, so you could generate a key, possibly store in our db, then use that.
A few options that warrant investigation 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend code enhancement New feature or request frontend OAM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants