Skip to content
This repository has been archived by the owner on Mar 24, 2018. It is now read-only.

Fix undefined name 'URLError' and use new style exceptions #223

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cclauss
Copy link

@cclauss cclauss commented Mar 12, 2018

flake8 testing of https://github.com/ros-infrastructure/buildfarm on Python 2.7.14

$ flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics

./buildfarm/storm.py:154:16: F821 undefined name 'URLError'
        except URLError, ex:
               ^
./buildfarm/storm.py:163:16: F821 undefined name 'URLError'
        except URLError, ex:
               ^
2     F821 undefined name 'URLError'

flake8 testing of https://github.com/ros-infrastructure/buildfarm on Python 2.7.14

$ __flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics__
```
./buildfarm/storm.py:154:16: F821 undefined name 'URLError'
        except URLError, ex:
               ^
./buildfarm/storm.py:163:16: F821 undefined name 'URLError'
        except URLError, ex:
               ^
2     F821 undefined name 'URLError'
```
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

This looks reasonable. Though this code base is basically legacy. (For example we're not using storm on demand anymore which this integrates with) We should consider testing out the new github Archive mode for it.

@cclauss
Copy link
Author

cclauss commented Mar 23, 2018

Is there a reason not to merge these changes before archiving?

@tfoote
Copy link
Member

tfoote commented Mar 23, 2018

In general I'd rather not merge this unless it's tested. And as far as I know the API that this is hitting has likely evolved since last tested so keeping this clearly as a snapshot from a few years ago better communicates the state of the system.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants