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

Removes id validation from MarathonGroup() #228

Merged
merged 3 commits into from
Oct 27, 2017
Merged

Removes id validation from MarathonGroup() #228

merged 3 commits into from
Oct 27, 2017

Conversation

daltonmatos
Copy link
Contributor

This validation was preventig the use of the root group (/), both from
MarathonGroup().from_json(<data>) and MarathonClient().get_group("/")

Fixes issue #227

This validation was preventig the use of the root group (`/`), both from
`MarathonGroup().from_json(<data>)` and `MarathonClient().get_group("/")`

Fixes issue #227
@daltonmatos
Copy link
Contributor Author

There are still flake8 errors, but not related to this modification.

@solarkennedy
Copy link
Contributor

Can you add a #noqa to get them to pass?

@daltonmatos
Copy link
Contributor Author

Sure, but there are only two errors:

./marathon/models/base.py:16:9: E722 do not use bare except'
./marathon/models/base.py:71:9: E722 do not use bare except'

The fix is simple and I can add it to this PR. What do you think?

This is the fix:

diff --git a/marathon/models/base.py b/marathon/models/base.py
index ecc8030..db77076 100644
--- a/marathon/models/base.py
+++ b/marathon/models/base.py
@@ -13,7 +13,7 @@ class MarathonObject(object):
     def __eq__(self, other):
         try:
             return self.__dict__ == other.__dict__
-        except:
+        except Exception:
             return False
 
     def __hash__(self):
@@ -68,7 +68,7 @@ class MarathonResource(MarathonObject):
     def __eq__(self, other):
         try:
             return self.__dict__ == other.__dict__
-        except:
+        except Exception:
             return False
 
     def __hash__(self):

Let me know and I will push a new commit.

Thanks,

E722: do not use bare except
@solarkennedy
Copy link
Contributor

Yes please, just fix it.

@daltonmatos
Copy link
Contributor Author

Done. Thanks,

@solarkennedy solarkennedy merged commit d68e5cc into thefactory:master Oct 27, 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.

2 participants