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

Implement uploading <project_uuid>.hazmapper to tapis. #54

Merged
merged 27 commits into from
Jun 4, 2021

Conversation

duckonomy
Copy link
Contributor

@duckonomy duckonomy commented Apr 8, 2021

Overview:

Implement saving the project file to tapis.

PR Status:

  • Ready.
  • Work in Progress.
  • Hold.

Related Jira tickets:

Summary of Changes:

Adds necessary route/service for the upload process and a new tapis interaction (create file and delete) in agave.py
Also added system_path, system_id, system_name to the project model for creating a link to designsafe projects

Testing Steps:

  1. Test with the HazMapper GUI (Add save to file functionality. hazmapper#50)

Notes:

I wasn't sure how to test the Tapis part. I've tried creating a mock like the agave_utils_with_geojson_file_mock but wasn't sure if that could update on making the call.

@duckonomy duckonomy requested review from nathanfranklin and reptillicus and removed request for nathanfranklin April 8, 2021 23:02
@reptillicus
Copy link
Contributor

It looks like there should also be a migration in here too? The additional fields in the project model will need one.

Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Just note two follow on things:

  • comment about logging
  • potentially deriving 'system_name' on the front end instead of having in model

geoapi/services/features.py Outdated Show resolved Hide resolved
geoapi/services/projects.py Outdated Show resolved Hide resolved
geoapi/services/projects.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

Just left one comment to consider and also for 3038e1d, is there a migration that goes along with it (as we have an early commit of this branch deployed on staging). 👍

geoapi/routes/public_projects.py Outdated Show resolved Hide resolved
@duckonomy duckonomy requested a review from nathanfranklin May 27, 2021 17:05
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

Awesome 💯

@nathanfranklin nathanfranklin merged commit 3ca7e24 into master Jun 4, 2021
@nathanfranklin nathanfranklin deleted the task/DES-1929-save-to-file branch May 19, 2022 16:55
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.

3 participants