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

docs: initial release #4

Merged
merged 1 commit into from Jun 16, 2017
Merged

Conversation

hjhsalo
Copy link
Contributor

@hjhsalo hjhsalo commented May 26, 2017

  • add initial documentation structure, with license authors, etc.
    (closes docs: initial release #1)
  • basic functionality to test client-server connection with ping

Signed-off-by: Harri Hirvonsalo harri.hirvonsalo@cern.ch

@hjhsalo
Copy link
Contributor Author

hjhsalo commented May 26, 2017

Note that #1 mentions a Dockerfile and this commit doesn't include one.

README.rst Outdated
==============
======================
REANA Server
======================
Copy link
Member

Choose a reason for hiding this comment

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

Any reason for having additional =? I remember that for the release checklist for Invenio we had to stick to the length of the tittle itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No reason. It's an error.

from flask import Flask

app = Flask(__name__)
app.secret_key = "hyper secret key"
Copy link
Member

Choose a reason for hiding this comment

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

🤣

Copy link
Contributor Author

Choose a reason for hiding this comment

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

¯_(ツ)_/¯
had to change something to make it more original :D


---
get:
summary: Ping the server. Responds with pong and HTTP 200
Copy link
Member

Choose a reason for hiding this comment

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

Responds with pong and HTTP 200

Isn't it a bit redundant if we already document all the responses below? It will be rendered along with the responses when we do the sphinxcontrib-openapi 🙃

@hjhsalo hjhsalo force-pushed the enhance-repo-structure branch 2 times, most recently from 5622fd9 to 8661a7e Compare May 26, 2017 15:37
@diegodelemos
Copy link
Member

Why is Dockerfile not included? We will need it to build the image and deploy the service. Moreover, adding the build to CI like here would be of great help.

# granted to it by virtue of its status as an Intergovernmental Organization or
# submit itself to any jurisdiction.

"""Flask application configuration."""
Copy link
Member

Choose a reason for hiding this comment

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

do we want to keep this empty file?

format='%(asctime)s - %(threadName)s - %(levelname)s: %(message)s'
)

app.config.from_object('config')
Copy link
Member

Choose a reason for hiding this comment

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

Loading empty config? do we need it?

url='https://github.com/reanahub/reana-server',
packages=['reana_server'],
zip_safe=False,
extras_require=extras_require,
Copy link
Member

Choose a reason for hiding this comment

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

install_requires=install_requires, missing


@app.route('/ping', methods=['GET'])
def ping(): # noqa
r"""Endpoint to ping the server. Responds with a pong.
Copy link
Member

Choose a reason for hiding this comment

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

Shall we plug apispec and sphinxcontrib-openapi already as here reanahub/reana-job-controller#40? We will need it if we want to show API docs...
If we do so, take into account pinning Sphinx see reanahub/reana-job-controller#43

Ping succeeded.
examples:
application/text:
pong
Copy link
Member

Choose a reason for hiding this comment

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

Usually we used to return OK, see for example http://opendata.cern.ch/ping

@hjhsalo hjhsalo force-pushed the enhance-repo-structure branch 3 times, most recently from db79f4e to d8e5d1d Compare June 8, 2017 13:24
@click.command()
@click.option('-n', '--name', default="Reana package",
help='Name of package/component', envvar='REANA_PACKAGE_NAME')
@click.option('-d', '--description', default="API description",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Better naming suggestions for these environment variables? e.g. PACKAGE_OAPI_VERSION is not that explanatory and without checking it out or without documentation (e.g. line comments) it is immediately clear what VERSION means in this context.

docs/restapi.rst Outdated
REST API
========

The REANA Server offers a REST API for management of workflow running in REANA
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if this would be s/workflow/analyses, or not specifying anything at all and just saying management of the REANA Cloud. Anyhow, not only workflows (understanding that analyses includes also single job/step tasks like https://github.com/reanahub/reana-demo-helloworld once we remove yadage)

errors='strict', lazy=False,
atomic=False) as output:
output.write(json.dumps(spec.to_dict(), indent=2, sort_keys=True))
output.close()
Copy link
Member

Choose a reason for hiding this comment

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

You can delete this since you are inside a context manager, can't you?

@hjhsalo hjhsalo force-pushed the enhance-repo-structure branch 9 times, most recently from 73c78c5 to 91f0db9 Compare June 14, 2017 13:20
@tiborsimko
Copy link
Member

@hjhsalo Have you had a look to the Travis CI failure?

Dockerfile Outdated
# granted to it by virtue of its status as an Intergovernmental Organization or
# submit itself to any jurisdiction.

FROM python:3.5-alpine
Copy link
Member

Choose a reason for hiding this comment

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

Elsewhere we are using FROM python:3.5.

It might be good to standardise on some base everywhere in order to avoid surprises, see for example why Elastic moved away from Alpine https://www.elastic.co/blog/docker-base-centos7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"It might be good to standardize on some base everywhere in order to avoid surprises"

There has been discussions about moving to Alpine in order to achieve smaller image sizes. The usage of python:3.5-alpine in reana-server is intentional. Motivation for this is to find out possible problems with the way current Dockerfile(s) of various REANA components are written.

I've seen and read the blog post in the past.

Dockerfile Outdated
chown -R reanauser /code
USER reanauser
EXPOSE 5000
CMD ["python", "reana_server/app.py"]
Copy link
Member

Choose a reason for hiding this comment

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

FLASK_APP=... and flask run?

run-tests.sh Outdated
pydocstyle reana_server && \
isort -rc -c -df **/*.py && \
FLASK_APP=reana_server/app.py python ./scripts/generate_openapi_spec.py && \
diff temp_openapi.json docs/openapi.json && \
Copy link
Member

Choose a reason for hiding this comment

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

diff -w should take care of the Travis CI problem


@click.command()
@click.option('-t', '--title',
help='Title of package/component exposing the API')
Copy link
Member

Choose a reason for hiding this comment

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

If people will copy/paste this code, as it were, then perhaps help=__title__ directly?

Copy link
Member

Choose a reason for hiding this comment

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

(And ditto for description, version, etc.)

@hjhsalo hjhsalo force-pushed the enhance-repo-structure branch 3 times, most recently from 0a54827 to 52d1ab8 Compare June 16, 2017 16:12
* add initial documentation structure, with license authors, etc.
  (closes reanahub#1)
* basic functionality to test client-server connection with ping

Signed-off-by: Harri Hirvonsalo <harri.hirvonsalo@cern.ch>
@tiborsimko tiborsimko merged commit e57ab25 into reanahub:master Jun 16, 2017
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.

docs: initial release
3 participants