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

Export GMS File In HydroShare Resource #2585

Merged
merged 2 commits into from
Jan 3, 2018
Merged

Conversation

arottersman
Copy link

@arottersman arottersman commented Jan 2, 2018

For mapshed projects include a GMS file
for each scenario

  • Named like scenario_this-is-a-scenario-name.gms
  • List of mapshed data is passed from client; gms
    file is created on the server before adding to
    hydroshare

Connects #2577

Demo

Project with exported GMS files

Testing Instructions

  • Create a mapshed project and confirm you can share and re-share it on hydroshare with both 1 and multiple scenarios
  • Confirm you can open the GMS files for each scenario
  • Create a site storm project and confirm you can still share it on hydroshare

@rajadain
Copy link
Member

rajadain commented Jan 2, 2018

I merged #2580 and haven't started taking a look at this yet, so feel free to squash and rebase on develop and I'll take a look then.

For mapshed projects include a GMS file
for each scenario

* Named like scenario_this-is-a-scenario-name.gms
* List of mapshed data is passed from client; gms
  file is created on the server before adding to
  hydroshare
@@ -12,6 +13,7 @@
from django.conf import settings

from apps.user.models import HydroShareToken
from apps.modeling.tasks import to_gms_file
Copy link
Member

Choose a reason for hiding this comment

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

Importing this here feels like it couples this too much with modeling. Some of this will be refactored in #2584, but I think it would be a better idea to use to_gms_file in views.py, and use it to construct a {name: 'scenario_xyz.gms', contents: '...'} object that is added to the files array given to hs.add_files. Does this make sense?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but it won't be as clean as described.to_gms_file returns a file, and shouldn't be passed as contents to hs.add_files which will try to rewrite it via StringIO.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right. Very well, we'll tackle that later.

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested. Exported current conditions and a new scenario with modifications to HydroShare. Both files made it, and were identical to respective exports from MMW.

@@ -19,6 +19,7 @@ var _ = require('lodash'),
modalIframeTmpl = require('./templates/iframeModal.html'),
vizerUrls = require('../settings').get('vizer_urls'),

GWLFE = 'gwlfe',
Copy link
Member

Choose a reason for hiding this comment

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

Can we import this from modeling/models? I'd prefer not defining the same constant in two places.

Copy link
Author

@arottersman arottersman Jan 3, 2018

Choose a reason for hiding this comment

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

This is in core which is usually agnostic to the other directories (which import it). I think instead it's time to move the model constants out into core/utils. We reference them in analyze, modeling, and now here.

I'll add a commit.

@arottersman
Copy link
Author

arottersman commented Jan 3, 2018

Added a commit to move the model constants into core/utils. I think this is ready to merge once the fixup is squashed. Let me know if you want to take another look.

@rajadain
Copy link
Member

rajadain commented Jan 3, 2018

Taking another look.

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested, working great!

We were using the model package constants in
analyze, modeling and core/modals, but they
were declared in modeling. Move them to
core utils so they are available throughout
the client.
@arottersman
Copy link
Author

Thanks for the review!

@arottersman arottersman merged commit 2e03597 into develop Jan 3, 2018
@arottersman arottersman deleted the arr/hydroshare-gms branch January 3, 2018 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants