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

Bugfix/working simple examples #542

Closed

Conversation

claweyenuk
Copy link
Contributor

@claweyenuk claweyenuk commented May 24, 2018

  • Any changes relevant to users are recorded in the CHANGELOG.md.
  • The documentation has been updated, if necessary.

I don't think this is mergeable, but I don't have the time to clean this up at the moment, particularly the keyjar code. But I thought someone might find this useful since there are issues like #212 and it is mentioned in the main README.

The steps in the README in here took some time to figure out as well, so hopefully that can help some people.

Note that I tested this using Python 3. I did run this under Python 2 and it required a few very bad hacks to get working in the wsgi lib that I think are because start response may have been called twice. I did not dig into that

@tpazderka
Copy link
Collaborator

Since the examples are SO broken, I am not against merging this with some stuff unfinished. Can you please update this to a current master as some stuff got already fixed?

@claweyenuk
Copy link
Contributor Author

Sure, I'll add some TODOs to the parts I don't quite understand and rebase

Thanks!

@claweyenuk claweyenuk force-pushed the bugfix/working-simple-examples branch from 5eeeee4 to 52bbe89 Compare June 6, 2018 23:44
@claweyenuk
Copy link
Contributor Author

I think this one is good (enough) to go now

@codecov-io
Copy link

Codecov Report

Merging #542 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #542   +/-   ##
=======================================
  Coverage   59.63%   59.63%           
=======================================
  Files          62       62           
  Lines       11247    11247           
  Branches     1981     1981           
=======================================
  Hits         6707     6707           
  Misses       3982     3982           
  Partials      558      558

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 8a9dfde...52bbe89. Read the comment docs.

@claweyenuk
Copy link
Contributor Author

The example README calls out #543 which I did not know how to work around

Copy link
Collaborator

@tpazderka tpazderka left a comment

Choose a reason for hiding this comment

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

Otherwise it is good.

@@ -1,4 +1,3 @@
oic==0.7.6
pyjwkest==1.0.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be included in the installation and is probably extra.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be worded "should not be included"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, sorry. Applies to both comments.

@@ -1,6 +1,5 @@
cherrypy==3.2.4
pyaml==15.03.1
Jinja2==2.7.3
oic==0.7.6
yubico-client==1.9.1
pyjwkest==1.0.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be included in the installation and is probably extra.

@tpazderka
Copy link
Collaborator

Merged manually.

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.

3 participants