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 sample tracking api endpoints #4872

Merged
merged 6 commits into from Nov 1, 2017

Conversation

Projects
None yet
4 participants
@guerler
Contributor

guerler commented Oct 26, 2017

This is a follow-up on #4526 and contains parts of #4493. Recently we removed the sample tracking controller endpoints with its 35 makos which cleared up the admin ui and our front-end structure. Removing it's legacy api endpoints will allow us to perform a similar cleanup for the backend. The sample tracking features are well embedded with corner-cases in several base classes and legacy form functions. There are about +3k lines of code which can be stripped from core. This PR is the first step towards that goal.

@guerler guerler added this to the 18.01 milestone Oct 26, 2017

@guerler guerler requested a review from martenson Oct 26, 2017

@martenson

This comment has been minimized.

Member

martenson commented Oct 26, 2017

  • I would like to have this officially deprecated for at least a release.
  • i would like to have feedback from people that were protective of the features in this API (e.g. @blankenberg I think)

Besides that, I am +1 on stripping this down to model (and just leaving the DB untouched).

@guerler

This comment has been minimized.

Contributor

guerler commented Oct 26, 2017

Did we not deprecate this feature in 17.09?

@martenson

This comment has been minimized.

@guerler

This comment has been minimized.

Contributor

guerler commented Oct 26, 2017

Ah I see.

@martenson

This comment has been minimized.

Member

martenson commented Oct 26, 2017

@guerler the release is not out yet (🙄), so we can change if we have consensus

@guerler

This comment has been minimized.

Contributor

guerler commented Oct 26, 2017

@galaxybot test this.

@guerler

This comment has been minimized.

Contributor

guerler commented Oct 27, 2017

I noticed that the deferred_job_queue object was removed from the job_manager in 9b7775b and I wonder if users could still be using the sample tracking feature in a reasonable fashion through the api without submitting deferred jobs, see (legacy?) call to queue: https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/webapps/galaxy/api/samples.py#L91.
ping @natefoo

@natefoo

This comment has been minimized.

Member

natefoo commented Oct 30, 2017

Deferred jobs were originally written to allow background transfers of data from sequencers, and were triggered internally via the web controllers. If they ever worked via the API I wasn't aware.

@guerler

This comment has been minimized.

Contributor

guerler commented Oct 30, 2017

Thanks, that makes sense and explains why I am seeing hints of it in the api.

@guerler guerler added status/review and removed status/WIP labels Oct 30, 2017

@jmchilton jmchilton merged commit 4485852 into galaxyproject:dev Nov 1, 2017

6 checks passed

api test Build finished. 306 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 162 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 57 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@jmchilton

This comment has been minimized.

Member

jmchilton commented Nov 1, 2017

Red is my favorite color!

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