Kitware / HPCCloud

A Cloud/Web-Based Simulation Environment
https://kitware.github.io/HPCCloud/
Apache License 2.0
50 stars 23 forks source link

Move files offline #561

Closed TristanWright closed 7 years ago

TristanWright commented 7 years ago

for #378

cjh1 commented 7 years ago

@TristanWright The result on my discussion on the Girder list is that we should add a custom endpoint to our plugin that would do a self.model('upload').moveFileToAssetstore(...) on a set of files that get passed to it. We would probably also want to add the option to pass in a ProgressContext object and update it with the count of bytes transferred, so that we can relay progress to the UI.

codecov-io commented 7 years ago

Current coverage is 61.77% (diff: 54.65%)

Merging #561 into master will decrease coverage by <.01%

@@             master       #561   diff @@
==========================================
  Files            59         61     +2   
  Lines          2789       2891   +102   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1723       1786    +63   
- Misses         1066       1105    +39   
  Partials          0          0          

Powered by Codecov. Last update a7b72a0...db74215

TristanWright commented 7 years ago

latest test uses 'moveFilesToAssetStore' my girder version is still 1.6, do I need to scrap vm to get it up to 2.1?

cjh1 commented 7 years ago

Should be able to do the following:

cd /opt/hpccloud/girder/
git pull origin master
girder-install web
TristanWright commented 7 years ago

Because the ProgressContext chunk size is so big there are about as many updates as there are files. Is there a way to adjust it? You can preview it right now if you'd like to see what I mean.

TristanWright commented 7 years ago

Also, my file move test is working on 2.7 but not 3.4?

cjh1 commented 7 years ago

Looks like you have some print statements that use the 2.7 syntax

 print tempfile.gettempdir()

They can probably be remove anyway?

TristanWright commented 7 years ago

I'm weak on python 2->3 differences, is there anything else that could be causing a failure?

cjh1 commented 7 years ago

We are using six to maintain compatibility between python 2.7 and 3. This is a good resource:

https://pythonhosted.org/six/

In this case you want to add a list( ) round your map. In 2.7 map returned a list, not in 3.

TristanWright commented 7 years ago

@cjh1 this is ready.

cjh1 commented 7 years ago

After I have moved a file I don't seem to be able to access the file contents. I receive the following error:

error

TristanWright commented 7 years ago

So the file is definitely moved, I can see it on the file system, the local assetstore is of filesystem type.

The girder exception is being thrown from here: https://github.com/girder/girder/blob/57231f3984e268822359d72320580dac439093e2/girder/utility/filesystem_assetstore_adapter.py#L261

@cjh1 let's look at this more tomorrow

cjh1 commented 7 years ago

@TristanWright So does the path not match what you are seeing on the filesystem?

TristanWright commented 7 years ago

The assetstore path does match and so does the file's path.

cjh1 commented 7 years ago

So if os.path.isfile is false what is the type of the path?

TristanWright commented 7 years ago

So with some logging I've found that the assetstore path is not being appended in when path is passed to fullPath()

TristanWright commented 7 years ago

because file.get('imported') is True

TristanWright commented 7 years ago

After the girder changes this is ready to merge, however I'm getting a ui lock when the file is loading.

screen shot 2017-01-19 at 11 43 32

Looking into this...

cjh1 commented 7 years ago

@TristanWright Things lock up when you try to load the moved file?

TristanWright commented 7 years ago

Any file, this is happening before the cancel request changes too, but I don't remember this ever happening before, that is I was able to test the cancel request changes. I'm running Chrome 55.0.2883.95

TristanWright commented 7 years ago

I'm wondering if it has to do with inserting 200KB of text to the DOM?

cjh1 commented 7 years ago

So this happens on master as well?

TristanWright commented 7 years ago

Yes I'm seeing it there too, are you? Basically when the file is loading I cannot close the modal.

cjh1 commented 7 years ago

Let me try

cjh1 commented 7 years ago

Works for me on master with a 249.8 KB file.

TristanWright commented 7 years ago

I've restarted chrome and it's fine now, very interesting. This can be merged.

cjh1 commented 7 years ago

Before we merge can we add to the test case check that we can download the file after its moved? This would catch the 'imported' issue we where seeing earlier in the week.

TristanWright commented 7 years ago

Def. I'll get append that to the current test

cjh1 commented 7 years ago

@TristanWright Tested this out, works well, great job! Two things:

TristanWright commented 7 years ago

Now that have cancellation (#545) I think we can add more cancel calls on umount?

You mean for other components that have some call? Yes, client.cancel() will cancel the most recent active request if there is any. When adding this I was wondering if it should be expanded to all requests, but I don't think we have many spots where we're doing simultaneous calls?

Will this move functionality work in the other workflows?

We should be able to integrate it. @jourdain is doing an overhaul of the workflow ui (#579) which will make it easier too.

cjh1 commented 7 years ago

OK, lets go ahead and merge, we can raise a follow up issue to ensure it works on other workflows