camelot-project / frontend

The frontend: The web-facing server software for CAMELOT
BSD 3-Clause "New" or "Revised" License
2 stars 14 forks source link

New features: store form data, better validation #145

Closed keflavich closed 9 years ago

keflavich commented 9 years ago

With this PR, all data submitted in the form will be included in the pull request in a separate JSON file. This will allow us to completely reconstruct any data submission later. That means we can allow other types of metadata to be submitted and validated, even if we don't require it.

This change required a corresponding change in the unit validation scheme to make it more flexible & powerful.

Reviews would be welcome!

agianne commented 9 years ago

Looks good to me.

e-koch commented 9 years ago

:+1:

keflavich commented 9 years ago

@e-koch @agianne thanks. This is a pretty big change, so could you give it a test locally? It would actually help to re-upload one of the data sets you've already uploaded so we can include the JSON data for those sets.

By test locally, I mean run the server locally and go all the way through the upload process.

keflavich commented 9 years ago

@e-koch thanks! I fixed both those issues.

agianne commented 9 years ago

@keflavich If I try to reupload my table I get this error:

Traceback (most recent call last): File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1836, in call return self.wsgi_app(environ, start_response) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1820, in wsgi_app response = self.make_response(self.handle_exception(e)) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1403, in handle_exception reraise(exc_type, exc_value, tb) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1817, in wsgi_app response = self.full_dispatch_request() File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1477, in full_dispatch_request rv = self.handle_user_exception(e) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1381, in handle_user_exception reraise(exc_type, exc_value, tb) File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1475, in full_dispatch_request rv = self.dispatch_request() File "/usr/local/lib/python2.7/dist-packages/flask/app.py", line 1461, in dispatch_request return self.view_functionsrule.endpoint File "/home/andrea/Software/CAMELOT/frontend/upload_form.py", line 567, in set_columns link_pull_database=link_pull_database, File "/usr/local/lib/python2.7/dist-packages/flask/templating.py", line 128, in render_template context, ctx.app) File "/usr/local/lib/python2.7/dist-packages/flask/templating.py", line 110, in _render rv = template.render(context) File "/usr/local/lib/python2.7/dist-packages/jinja2/environment.py", line 969, in render return self.environment.handle_exception(exc_info, True) File "/usr/local/lib/python2.7/dist-packages/jinja2/environment.py", line 742, in handle_exception reraise(exc_type, exc_value, tb) File "/home/andrea/Software/CAMELOT/frontend/templates/show_plot.html", line 1, in top-level template code {% extends 'base.html' %} File "/home/andrea/Software/CAMELOT/frontend/templates/base.html", line 78, in top-level template code {% block body %} File "/home/andrea/Software/CAMELOT/frontend/templates/show_plot.html", line 76, in block "body" {% if imagename[:-3] == 'png' %} TypeError: 'NoneType' object has no attribute 'getitem'

but the PRs were created anyway!

keflavich commented 9 years ago

@agianne OK, thanks - this indicates some sort of error with the plotter. I don't understand it right away. It looks like the variable "imagename" is never passed to show_plot.html from the traceback; could you investigate this? Any clues would be helpful. It may be a separate issue; do you get the same if you try uploading to the master camelot?

agianne commented 9 years ago

@keflavich Doing the same from the app results in

No difference was detected between the input branch of uploads/ ard the master after staging. >Possibly the submitted data file contains no data.

I will investigate why these problems arise.

keflavich commented 9 years ago

@agianne I can also reproduce the error, but I still don't understand why it's occurring. I'm investigating.

keflavich commented 9 years ago

The latest commit should allow you to run the tests, but please do so with caution - each time the test is run, it creates a new commit and e-mails anyone subscribed to the uploads and database repositories. I've added a new testmode='skip' option to allow you to skip the pull request phase.

keflavich commented 9 years ago

@agianne I believe the error you saw should be fixed now.

agianne commented 9 years ago

Ok, it now works locally!

e-koch commented 9 years ago

The tests pass for me, but I get this error when trying to upload the ErikRosolowsky data:

Error: the remote URL git@github.com:camelot-project/uploads (which is really 'upl') does not match the expected one 'uploads'

Traceback (most recent call last): File "upload_form.py", line 502, in set_columns testmode=testmode) File "upload_form.py", line 560, in create_pull_request branch=branch_database, File "upload_form.py", line 626, in setup_submodule .format(check_upstream, database, name)) Exception: Error: the remote URL git@github.com:camelot-project/uploads (which is really 'upl') does not match the expected one 'uploads'

Is this just my local version?

agianne commented 9 years ago

@e-koch What do you have in .gitmodules?

e-koch commented 9 years ago
[submodule "database"]
        path = database
        url = git@github.com/camelot-project/database.git
[submodule "uploads"]
        path = uploads
        url = git@github.com/camelot-project/uploads.git
agianne commented 9 years ago

@e-koch No, that's ok... We had a similar issue when the ".git" was missing from "url = git@github.com/camelot-project/uploads.git"

e-koch commented 9 years ago

@agianne - I get the same error when trying to locally upload using the current master branch. So this is just something wonky in my local version.

@keflavich - Tests pass, so as far as I can tell it is good to go!

keflavich commented 9 years ago

@e-koch I don't know exactly what's going on but I tried making the error message more verbose