Skip to content

Support for dict in Client.parse_response formats #790

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

Merged
merged 5 commits into from
Jul 7, 2021

Conversation

Hultner
Copy link
Contributor

@Hultner Hultner commented Jul 5, 2021

  • Any changes relevant to users are recorded in the CHANGELOG.md.
  • The documentation has been updated, if necessary.
  • New code is annotated.
  • Changes are covered by tests.

In response to #789

Sorry, something went wrong.

Copy link
Collaborator

@schlenk schlenk left a comment

Choose a reason for hiding this comment

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

It seems line 630ff in parse_response() needs some cast() to make mypy happy for the urlencoded path still.

        if sformat == "urlencoded":
            str_info = cast(str, info)
            info = self.get_urlinfo(str_info)

Looks good otherwise. Could you add this please?

@Hultner
Copy link
Contributor Author

Hultner commented Jul 7, 2021

It seems line 630ff in parse_response() needs some cast() to make mypy happy for the urlencoded path still.

        if sformat == "urlencoded":
            str_info = cast(str, info)
            info = self.get_urlinfo(str_info)

Looks good otherwise. Could you add this please?

Fixed it now. I also added an additional type guard so that an error is raised if info is passed as an dict but sformat isn't a dictionary.

Temporarily raise mccabe limits.
@codecov-commenter
Copy link

Codecov Report

Merging #790 (f62d24c) into master (a5e8986) will decrease coverage by 0.00%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #790      +/-   ##
==========================================
- Coverage   63.69%   63.68%   -0.01%     
==========================================
  Files          64       64              
  Lines       11843    11846       +3     
  Branches     2095     2096       +1     
==========================================
+ Hits         7543     7544       +1     
- Misses       3704     3705       +1     
- Partials      596      597       +1     
Impacted Files Coverage Δ
src/oic/oauth2/__init__.py 69.29% <60.00%> (-0.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5e8986...f62d24c. Read the comment docs.

Copy link
Collaborator

@schlenk schlenk left a comment

Choose a reason for hiding this comment

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

Seems the extra isinstance() check pushes pylama over the threshold of its complexity measure due to the extra if. I just pushed the limit a little higher as it is arbitrary anyway and will refactor it later.

The Travis check is basically dead (due to travis stopping its free service), so no issue there.

@schlenk schlenk merged commit 7cf85cd into CZ-NIC:master Jul 7, 2021
@Hultner Hultner deleted the feature/allow-dict-encoding branch July 7, 2021 12:40
@Hultner
Copy link
Contributor Author

Hultner commented Jul 7, 2021

Seems the extra isinstance() check pushes pylama over the threshold of its complexity measure due to the extra if. I just pushed the limit a little higher as it is arbitrary anyway and will refactor it later.

The Travis check is basically dead (due to travis stopping its free service), so no issue there.

Thanks for the swift feedback and process :)

I saw that the last release was last year. Is there any plan for when the next release will be? I've seen some other changes in master which aren't in the released version which I'd like to have.

@schlenk
Copy link
Collaborator

schlenk commented Jul 7, 2021

Yes, a release is planned soonish, i think when @tpazderka finds the time to do one.

@Hultner
Copy link
Contributor Author

Hultner commented Jul 7, 2021

Nice, great to hear. Looks like you've made some python 3.9 improvements since the last release.

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.

None yet

3 participants