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

Remove legacy library interface #4908

Merged
merged 35 commits into from Nov 13, 2017

Conversation

4 participants
@guerler
Contributor

guerler commented Nov 1, 2017

This PR removes the legacy library interface. The main challenge is that the api controller uses the web controller functions. This PR attempts to unwind the two. While unwinding it turns out that many of the validation checks, parameters and code paths used through the controller are not working/not used when called through the API. Access validation e.g. is tested through the api but the result is not used to prohibit the operation, see: https://github.com/galaxyproject/galaxy/pull/4908/files#diff-a74fb51de19ecbcb13be07dea19fde07R316.

@martenson

This comment has been minimized.

Member

martenson commented Nov 1, 2017

xref: #1995

@martenson martenson added this to WIP in Data Libraries Nov 1, 2017

@martenson martenson self-requested a review Nov 1, 2017

@martenson

This comment has been minimized.

Member

martenson commented Nov 1, 2017

I signed up as a reviewer for this since I am interested and working on related code atm.

@guerler guerler added status/review and removed status/WIP labels Nov 3, 2017

@galaxybot galaxybot added this to the 18.01 milestone Nov 3, 2017

guerler and others added some commits Nov 9, 2017

Restore database/info.txt.
It was problematically removed with a recent PR and is causing Travis breakages - Galaxy assumes this directory exists.
@martenson

This comment has been minimized.

Member

martenson commented Nov 10, 2017

When adding datasets from user directory:

galaxy.web.framework.decorators ERROR 2017-11-10 12:22:58,257 Uncaught exception in exposed API method:
Traceback (most recent call last):
  File "/Users/marten/devel/git/galaxy/lib/galaxy/web/framework/decorators.py", line 281, in decorator
    rval = func(self, trans, *args, **kwargs)
  File "/Users/marten/devel/git/galaxy/lib/galaxy/webapps/galaxy/api/library_datasets.py", line 480, in load
    trans, 'api', kwd, os.path.basename(file), file, 'server_dir', library_bunch))
  File "/Users/marten/devel/git/galaxy/lib/galaxy/webapps/galaxy/api/library_datasets.py", line 568, in _make_library_uploaded_dataset
    link_data_only = params.get('link_data_only', 'copy_files')
AttributeError: 'str' object has no attribute 'get'
@martenson

This comment has been minimized.

Member

martenson commented Nov 10, 2017

also errors when using import directory or path

Traceback (most recent call last):
  File "/Users/marten/devel/git/galaxy/lib/galaxy/web/framework/decorators.py", line 281, in decorator
    rval = func(self, trans, *args, **kwargs)
  File "/Users/marten/devel/git/galaxy/lib/galaxy/webapps/galaxy/api/library_datasets.py", line 495, in load
    trans, kwd, library_bunch, 200, '')
  File "/Users/marten/devel/git/galaxy/lib/galaxy/webapps/galaxy/api/library_datasets.py", line 521, in _get_path_paste_uploaded_datasets
    uploaded_datasets.append(self._make_library_uploaded_dataset(trans, 'api', params, name, path, 'path_paste', library_bunch, folder))
TypeError: _make_library_uploaded_dataset() takes at most 8 arguments (9 given)

@guerler guerler added status/review and removed status/WIP labels Nov 10, 2017

@martenson

This comment has been minimized.

Member

martenson commented Nov 10, 2017

My branch was busted, with a clean checkout the errors above are not thrown anymore.

@martenson

Looks well, works well. Thanks Aysam.

@martenson martenson merged commit d2a5695 into galaxyproject:dev Nov 13, 2017

0 of 7 checks passed

api test Test started.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
framework test Test started.
Details
integration test Test started.
Details
lgtm analysis: JavaScript Fetching Git Commits
Details
selenium test Test started.
Details
toolshed test Test started.
Details
@guerler

This comment has been minimized.

Contributor

guerler commented Nov 13, 2017

Thanks a lot for the review man. I know it was quite a bit of work.

@martenson martenson moved this from WIP to Closed in Data Libraries Nov 14, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 13, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 13, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 14, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 15, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 15, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 15, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 18, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 18, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 19, 2017

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Dec 20, 2017

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