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

Removes Sample request and External services makos/controllers #4526

Merged
merged 30 commits into from Oct 25, 2017

Conversation

Projects
None yet
3 participants
@guerler
Copy link
Contributor

commented Aug 30, 2017

This PR is branched from #4480 and contains sections from #4493. It removes the controller endpoints and associated makos for sample requests and external services. These are features being deprecated in 17.09. Additionally it reorganizes the left admin panel by reducing it to three categories: Server, User Management and Tools/Toolshed.

@martenson martenson referenced this pull request Aug 30, 2017

Closed

deprecations for 17.09 #3280

guerler added some commits Aug 31, 2017

@guerler guerler added status/review and removed status/WIP labels Sep 13, 2017

@guerler guerler requested a review from martenson Sep 14, 2017

@erasche

This comment has been minimized.

Copy link
Member

commented Sep 25, 2017

Works fine / looks good to me. Only comment is with the newer, shorter menu I see a white section at the bottom.

auswahl_066

It looks like this can be fixed with

diff --git a/client/galaxy/style/less/base.less b/client/galaxy/style/less/base.less
index fb998e414c..49768ad355 100644
--- a/client/galaxy/style/less/base.less
+++ b/client/galaxy/style/less/base.less
@@ -134,6 +134,7 @@ body  {
     width: 250px;
     z-index: 200;
     border-right: solid @layout-border-color 1px;
+    background: @panel-bg-color;
 }
 #left-border  {
     left: 250px;
@martenson

This comment has been minimized.

Copy link
Member

commented Sep 25, 2017

@erasche that is unrelated to this PR I think, I saw it before.

@erasche

This comment has been minimized.

Copy link
Member

commented Sep 25, 2017

👍 from me (but not merging due to requested review from @martenson)

@martenson

This comment has been minimized.

Copy link
Member

commented Sep 25, 2017

We need to have another discussion about this I think. There were comments about how/if to proceed with the controllers removal or whether to just bind them to the tracking configuration.

@guerler guerler closed this Sep 29, 2017

@guerler guerler force-pushed the guerler:remove_samples_000 branch from cae0886 to 73bb4ba Sep 29, 2017

guerler added some commits Oct 17, 2017

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Oct 18, 2017

ping @martenson. Can we discuss this? It would be nice if we could move forward. It could allow us to complete the mako-to-js transition within this year. Also no one responded to the email we sent out a couple weeks ago.

@martenson

This comment has been minimized.

Copy link
Member

commented Oct 22, 2017

@guerler yep, let's talk monday

martenson added some commits Oct 23, 2017

tie sample tracking api controllers functionality to config
introduce enable_legacy_sample_tracking_api config option that controls that
disable by default
@martenson

This comment has been minimized.

Copy link
Member

commented Oct 23, 2017

I disabled both samples.py and requests.py API controllers unless new config option is set to True (False by default)

#enable_legacy_sample_tracking_api = False

I think this is now ready for merging and I will do so later today, unless there are objections.

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Oct 23, 2017

We got some new failing tests.

@martenson

This comment has been minimized.

Copy link
Member

commented Oct 23, 2017

The api tests were already failing for https://jenkins.galaxyproject.org/job/docker-api/9020/ which is an unrelated PR 🤔

@martenson

This comment has been minimized.

Copy link
Member

commented Oct 24, 2017

switching to WIP, found few things that need to be included

@martenson

This comment has been minimized.

Copy link
Member

commented Oct 24, 2017

seems the problems were caused by bad merges, @guerler is on it

@guerler guerler force-pushed the guerler:remove_samples_000 branch from 883bf82 to 8f6e5eb Oct 24, 2017

@guerler guerler removed the status/WIP label Oct 24, 2017

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Oct 24, 2017

@galaxybot test this.

@martenson martenson merged commit a1f0895 into galaxyproject:dev Oct 25, 2017

5 of 6 checks passed

api test Build finished. 304 tests run, 4 skipped, 10 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
@martenson

This comment has been minimized.

Copy link
Member

commented Oct 25, 2017

Thanks for your work @guerler!

@guerler

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2017

Thanks a lot for the help and review @martenson.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.