astrofrog/acknowledgment-generator

LBT error in Python 3.5

benjaminrose opened this issue · 14 comments

Looking at Travis CI, it looks like the ":" in the LBT acknowledgment is giving the Python 3.5 test issues. Since the web page is working, should we just drop Python 3.5 fromTravis?

Where did you see that the LBT acknowledgment was creating an issue? To me, travis job says it all:

The command "if [[ $TRAVIS_PYTHON_VERSION == 3.5 && $TRAVIS_BRANCH == "gh-pages" ]]; then git checkout gh-pages; git add database.json; git commit -m "Updated database.json"; git push -q https://astrobot:$GITHUB_API_KEY@github.com/astrofrog/acknowledgment-generator gh-pages >& /dev/null; fi" exited with 128.

Source

I am new to Travis and not familiar with its error messages. I assumed it as and LBT error since that was the last "working correctly" line and it has an extra ":" that usually needs to be quoted in a .yaml file.

It appears that it is not an issue with LBT, so what do we need to fix so that Python 3.5 does not always fail?

Evil is inside the details. Just below the last line you mention it says in green The command "python parse_entries.py" exited with 0.. Hence the error is not in the parsing. As for the escaping of : it is not always necessary, as documented in YAML specs. But more easily you can see here that the LBT yaml is valid. Moreover, as a general rule, when a problem pops up, it's usually better to fix it rather than suppressing the test...

So what fails is this tricky command. I'll see if I can play with it in my fork.

I don't have the hands on the .travis-ci.yaml file to play with. But the file in it must be improved and simplified (what's the point of running it against 3 versions of python?!). @astrofrog should have a look.

I am learning more about Travis CI and it looks like the issue is with updating the Astrobot updating database.json back to gh-pages, lines 22 through 27.

I feel like this line should not be running on pull requests. Maybe things are running as desired, but seeing failed tests on a PR just because it did not automatically ship seems like not the best solution.

The idea is good, but the process sucks anyway. It makes no sense to ask people to create YAML files to later on parse them to create a JSON database. Project like this allow to read YAML directly. However, you cannot AFAIK get a list of existing local files in javascript. Hence, you cannot retrieve the list of all YAML files in a directory dynamically. To me, when one needs to handle user input, one would be better of having a small (and free) real webserver with a nice webpage accepting new entries.

Sorry for the silence, I do plan to reply to this thread soon!

Busy with that? http://docs.astropy.org/en/stable/whatsnew/2.0.html Perfectly understandable! I wish I could find a small door to contribute as well some time. 👍

I am learning more about Travis CI and it looks like the issue is with updating the Astrobot updating database.json back to gh-pages, lines 22 through 27.

@benjaminrose - those lines shouldn't run on a PR except if (as I just realized) the user makes a gh-pages branch too and opens a pull request from that.

what's the point of running it against 3 versions of python?!

To make sure that the parsing script remains compatible with them, but the upload itself is only done from 3.5.

I'm investigating the failure btw

Ok so indeed the issue is that I think Travis changed the meaning of $TRAVIS_BRANCH and so now it was running at if statement even for pull requests.

To me, when one needs to handle user input, one would be better of having a small (and free) real webserver with a nice webpage accepting new entries.

I agree, but the current system is actually pretty low maintenance apart from issues like this, and makes it easy for people to contribute and fix contributions, so I'd like to keep it as-is for now.

If I wasn't about to start some weeks of vacations, I'd say "challenge accepted". :-) Fair enough, thanks for the update.