Skip to content

Conversation

@awwad
Copy link
Contributor

@awwad awwad commented Aug 14, 2018

Replaces PR #769.

This updates the project's dependencies. See #769 for changelogs.

Currently, this results in some linter warnings. I'll comb through those.

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@awwad awwad self-assigned this Aug 14, 2018
@awwad
Copy link
Contributor Author

awwad commented Aug 14, 2018

pylint 1.9.3 -> 2.0.0 jump results in some linter warnings

@awwad
Copy link
Contributor Author

awwad commented Aug 14, 2018

A try-except-raise checker was added to pylint in 2.0.0 (originally in this commit), raising a warning (error downgraded to warning in 2.1) if there's an immediate re-raise in an except, outside of some special cases.

Here's one of the offending constructions (or see it in context here)

        try:
          # ....
        # Unknown role, re-raise exception.
        except tuf.exceptions.UnknownRoleError:
          raise
        ## Note: no further except or else clauses!

It's not clear to me why this construction is useful, beyond the guess that it might indicate to a reader that an exception of a certain type is expected to occur here in certain situations.... That alone doesn't seem worth the extra bit of code and the wrinkle in the exception stack, though.

Is anyone aware of a good reason to keep a construction like the above?
@alyptik @SantiagoTorres
(Note that if there are further except clauses, there are reasons to have an immediate re-raise -- see pylint 2323 -- the case of excluding a subclass re-raise from a broader exception handling clause. Incidentally, this would still be something to avoid, but it does make some sense....)

awwad added a commit that referenced this pull request Aug 14, 2018
These changes simplify logic, removing some try/except structures
that were unnecessary and potentially confusing, and get us back
to passing pylint's test.

pylint 2.0.0 adds try-except-raise tests, to catch immediate
re-raising after catching an exception, outside of some special
cases. See this GitHub comment for more info:
#770 (comment)

Signed-off-by: Sebastien Awwad <[email protected]>
@awwad
Copy link
Contributor Author

awwad commented Aug 14, 2018

PR #771 fixes the code construction issues pylint > 2.0.0 finds. Since this PR does not introduce any of those issues, it doesn't need to wait to be merged until after 771.

@awwad
Copy link
Contributor Author

awwad commented Aug 14, 2018

As best I can tell, updating these dependencies doesn't break anything, so I could use a review at this point and I'll add sign-off and merge without the linter passing (see above comments).

@awwad awwad requested review from trishankkarthik and removed request for trishankkarthik August 14, 2018 20:37
@awwad
Copy link
Contributor Author

awwad commented Aug 14, 2018

@trishankatdatadog, can you tell me if any of these dependency changes would be an issue for you?

@trishankatdatadog
Copy link
Contributor

Updating cryptography shouldn't bother us. Thanks for checking!

@awwad awwad requested a review from SantiagoTorres August 14, 2018 21:10
Update astroid from 1.6.5 to 2.0.4
Update cryptography from 2.2.2 to 2.3
Update cryptography from 2.2.2 to 2.3
Update gitdb2 from 2.0.3 to 2.0.4
Update gitpython from 2.1.10 to 2.1.11
Update pbr from 4.0.4 to 4.2.0
Update pluggy from 0.6.0 to 0.7.1
Update py from 1.5.3 to 1.5.4
Update pylint from 1.9.2 to 2.1.1
Update pyyaml from 3.12 to 3.13
Update smmap2 from 2.0.3 to 2.0.4
Update stevedore from 1.28.0 to 1.29.0
Update tox from 3.0.0 to 3.2.1

Signed-off-by: Sebastien Awwad <[email protected]>
@awwad awwad force-pushed the update_dependencies_2018_08 branch from 873b00c to 3602d61 Compare August 14, 2018 21:19
@awwad awwad merged commit c690096 into develop Aug 15, 2018
awwad added a commit that referenced this pull request Aug 21, 2018
These changes simplify logic, removing some try/except structures
that were unnecessary and potentially confusing, and get us back
to passing pylint's test.

pylint 2.0.0 adds try-except-raise tests, to catch immediate
re-raising after catching an exception, outside of some special
cases. See this GitHub comment for more info:
#770 (comment)

Signed-off-by: Sebastien Awwad <[email protected]>
@awwad awwad deleted the update_dependencies_2018_08 branch November 5, 2018 21:58
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.

4 participants