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

consumer: addition of JobStatusConsumer class #106

Merged
merged 1 commit into from
Jul 11, 2018

Conversation

dinosk
Copy link
Member

@dinosk dinosk commented Jul 4, 2018

  • ADD JobStatusConsumer class, which inherits from the
    base Consumer class in reana-workflow-commons and sets
    it to consumer from the jobs-status queue.

Depends on reanahub/reana-workflow-commons#2
Addresses #103

Signed-off-by: Dinos Kousidis dinos.kousidis@cern.ch

@dinosk dinosk self-assigned this Jul 4, 2018
@diegodelemos diegodelemos added this to the 0.3.0 milestone Jul 9, 2018
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-
#
# This file is part of REANA.
# Copyright (C) 2018 CERN.
# Copyright (C) 2017, 2018 CERN.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2018 only?

@@ -20,39 +20,23 @@
# granted to it by virtue of its status as an Intergovernmental Organization or
# submit itself to any jurisdiction.

"""REANA Workflow Controller MQ Consumer."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing module docstring.

@dinosk dinosk force-pushed the job-status-consumer branch 2 times, most recently from 129a451 to 6af9bd9 Compare July 10, 2018 10:05
Dockerfile Outdated
@@ -24,8 +24,6 @@ RUN apt-get update && \
apt-get install -y vim-tiny && \
pip install --upgrade pip

RUN pip install -e git://github.com/reanahub/reana-commons.git@master#egg=reana-commons
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing reana-commons. You can add it just before COPY . /code, also reana-workflow-commons could be moved before.

Copy link
Member Author

@dinosk dinosk Jul 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding it, in reanahub/reana-workflow-engine-cwl#46 @tiborsimko suggested adding it after the COPY . /code, I am not sure if this specific order affects the rebuilding though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because until we don't release reana-commons and reana-workflow-commons in PyPI we will have to rebuild the image with no cache if we change any of the repos since the docker builder will take it as cached. You can move them after and once we release on PyPI we remove the lines, I just made the comment because it is pulling from Github the two repos and installing them every time I rebuild, therefore slowing down the rebuild.

Lets follow @tiborsimko advise then:

I would propose the safest order like:
pypi requirements builder all to cache
COPY . /code
pip r-commons r-w-commons
pip install .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was referring to the order 1. COPY 2. pip install -e git://... if it affected the docker image building, the rest is of course obvious. The order is already as described

@dinosk dinosk force-pushed the job-status-consumer branch 5 times, most recently from b7f4d41 to 08ecf72 Compare July 10, 2018 12:41
* ADD JobStatusConsumer class, which inherits from the
  base Consumer class in reana-workflow-commons and sets
  it to consumer from the jobs-status queue.

Signed-off-by: Dinos Kousidis <dinos.kousidis@cern.ch>
@@ -23,7 +23,7 @@ FROM python:3.6
RUN apt-get update && \
apt-get install -y vim-tiny && \
pip install --upgrade pip

RUN pip install -e git://github.com/reanahub/reana-workflow-commons.git@master#egg=reana-workflow-commons
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 2 pip install from github had to be moved here, before the installation of requirements-builder, as reana-commons and reana-workflow-commons need to be preset in the setup.py, and it fails otherwise.

@diegodelemos diegodelemos merged commit b77bc13 into reanahub:master Jul 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants