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

Fix for missing included files on history export and import. #417

Merged
merged 2 commits into from Oct 22, 2015

Conversation

Projects
None yet
4 participants
@vimalkvn
Contributor

vimalkvn commented Jul 2, 2015

Related Trello card: https://trello.com/c/oq1ASbkC

Notes:

If a history was exported previously, galaxy would just serve the cached version.
In that case, it is better to make some changes to the history for example, by re-running
the last job and then exporting it.

Export of histories that haven't been exported previously should work without problems.

Fix for missing included files on history export and import.
Notes:

If a history was exported previously, galaxy would just serve the cached version.
In that case, it is better to make some changes to the history for example, by re-running
the last job and then exporting it.

Export of histories that haven't been exported previously should work without problems.

@nsoranzo nsoranzo added the kind/bug label Jul 3, 2015

@@ -197,6 +197,24 @@ def get_tag_str( tag, value ):
raise Exception( "Invalid dataset path: %s" % temp_dataset_file_name )
if datasets_usage_counts[ temp_dataset_file_name ] == 1:
shutil.move( temp_dataset_file_name, hda.file_name )

This comment has been minimized.

@natefoo

natefoo Jul 14, 2015

Member

This is pre-existing code but I assume it informed the code below. It should use the object store, e.g.:

self.app.object_store.update_from_file( hda.dataset, file_name=temp_dataset_file_name, create=True )
try:
dataset_extra_files_path = dataset_attrs[ 'extra_files_path' ]
except KeyError:
pass

This comment has been minimized.

@natefoo

natefoo Jul 14, 2015

Member

This would be more concise:

dataset_extra_files_path = dataset_attrs.get( 'extra_files_path', None )
except OSError:
file_list = []
if len( file_list ):

This comment has been minimized.

@natefoo

natefoo Jul 14, 2015

Member

if file_list: is also fine here too (because [] == False)

if len( file_list ):
os.mkdir( hda.extra_files_path )
for extra_file in file_list:
shutil.move( os.path.join( archive_dir, dataset_extra_files_path, extra_file ), hda.extra_files_path )

This comment has been minimized.

@natefoo

natefoo Jul 14, 2015

Member

self.app.object_store.update_from_file( hda.dataset, extra_dir='dataset_%s_files' % hda.dataset.id, alt_name=extra_file, file_name=os.path.join( archive_dir, dataset_extra_files_path, extra_file ), create=True)

@vimalkvn

This comment has been minimized.

Contributor

vimalkvn commented Jul 20, 2015

Thanks Nate. I have made these changes now.

@martenson

This comment has been minimized.

Member

martenson commented Oct 19, 2015

@natefoo would you please re-review this given the requested changes were made, thanks

@natefoo

This comment has been minimized.

Member

natefoo commented Oct 22, 2015

Thanks @vimalkumarvelayudhan, tested and this looks/works great!

natefoo added a commit that referenced this pull request Oct 22, 2015

Merge pull request #417 from vimalkumarvelayudhan/dev
Fix for missing included files on history export and import.

@natefoo natefoo merged commit 53f924f into galaxyproject:dev Oct 22, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@vimalkvn

This comment has been minimized.

Contributor

vimalkvn commented Oct 22, 2015

Glad to hear that! Thanks @natefoo

@natefoo

This comment has been minimized.

Member

natefoo commented Oct 22, 2015

Sorry it took me so long to check it out. Thanks for contributing!

@vimalkvn

This comment has been minimized.

Contributor

vimalkvn commented Oct 22, 2015

No problem!

@martenson

This comment has been minimized.

Member

martenson commented Oct 22, 2015

@natefoo what do you think about this targeting 15.10 ?

@natefoo

This comment has been minimized.

Member

natefoo commented Oct 22, 2015

Yeah, definitely. I'll open a PR

@martenson

This comment has been minimized.

Member

martenson commented Oct 22, 2015

Thank you for the contribution @vimalkumarvelayudhan !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment