-
Notifications
You must be signed in to change notification settings - Fork 80
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
Adding finalized skeleton of web pipeline 👍ready to be merged👍 #117
Conversation
|
||
commands = [] | ||
for data_type, command in split: | ||
print "INPUT: ", data_type, command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this and all the following print statements?
👍 |
@@ -17,11 +17,8 @@ install: | |||
- pip install . | |||
script: | |||
- qiita_env make_test_env | |||
- nosetests --with-doctest | |||
- nosetests --with-doctest --with-coverage --cover-package=coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@squirrelo can you undo this?
This should be ready for merge! The instructions I suggest are to pull down this branch then: qiita_env drop_demo_env
qiita_env make_demo_env # you need an internet connection for this step
python qiita_pet/webserver.py In a browser go to http://localhost:8888 and then create a new meta-analysis, it should take you through the end of the workflow. |
|
||
class AnalysisWaitHandler(BaseHandler): | ||
@authenticated | ||
def get(self, analysis_id): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, with this implementation, people will be able to change the id in the URL to whatever they want and retrieve that analysis, whether or not they are supposed to have access to it. See #147
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: this is fine for the demo, but we should address it in the future
self.r_server = Redis() | ||
self.redis = Client() | ||
self.redis.connect() | ||
# self.redis = Client() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these lines here because we will use them after the demo, but don't want to use them right now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's why.
@antgonza gave his blessing for this PR to be merged, if all is good someone else should merge this. |
Yep, all looks good to me! |
Adding finalized skeleton of web pipeline ready to be merged
Still can't show results or have a proper working wait page, as those are dependent on the job running working, but you can now go through the entire pipeline and see how it looks.