Adding Flask_pyoidc and Updating to Python 3.6 #169

devinmatte · 2018-03-03T02:43:47Z

Added flask_oidc

This removes the need for the overarching oidc that causes timeout issues.

I updated to Python 3.6 in order to get that package to work properly, as well as prepare to upgrade ldap in the coming week

Should fix #90

liam-middlebrook

Overall I think that the structure after your changes are applied is an improvement.

I've left quite a few comments on places where changes under the "Adding flask_pyoidc" commit d214bdf could be moved to a style (or cosmetic) change commit.

Keeping cosmetic changes separate from functional changes allows for easier reviewing and bisecting of commits.

liam-middlebrook · 2018-03-03T04:10:34Z

conditional/__init__.py

-from flask import Flask, redirect, request, render_template, g
-from flask_sqlalchemy import SQLAlchemy
-from flask_migrate import Migrate
+


Move to the style fixes commit.

Also why is this being reordered (and some lines being deleted)?

Based on: https://www.python.org/dev/peps/pep-0008/#imports
The lines being deleted are unused imports

Then that should be a separate commit from adding pyoidc support.

liam-middlebrook · 2018-03-03T04:11:28Z

conditional/__init__.py

 # pylint: disable=C0413
 from conditional.models.models import UserLog

+


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:11:32Z

conditional/__init__.py

 # Configure Logging
-def request_processor(logger, log_method, event_dict): # pylint: disable=unused-argument, redefined-outer-name
+def request_processor(logger, log_method, event_dict):  # pylint: disable=unused-argument, redefined-outer-name


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:11:41Z

conditional/__init__.py

@@ -52,7 +58,7 @@ def request_processor(logger, log_method, event_dict): # pylint: disable=unused-
    return event_dict


-def database_processor(logger, log_method, event_dict): # pylint: disable=unused-argument, redefined-outer-name
+def database_processor(logger, log_method, event_dict):  # pylint: disable=unused-argument, redefined-outer-name


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:12:56Z

conditional/__init__.py


-from conditional.blueprints.dashboard import dashboard_bp # pylint: disable=ungrouped-imports
+from conditional.blueprints.dashboard import dashboard_bp  # pylint: disable=ungrouped-imports


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:27:15Z

conditional/blueprints/housing.py

 from flask import Blueprint, request, jsonify

+from conditional import db, auth


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:27:41Z

conditional/blueprints/intro_evals.py

-from conditional.util.ldap import ldap_get_intro_members
-
-from conditional.models.models import FreshmanCommitteeAttendance
+from conditional import start_of_year, auth


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:28:09Z

conditional/blueprints/logs.py

 from flask import Blueprint, request

+from conditional import auth


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:29:29Z

conditional/blueprints/slideshow.py

-
-from conditional import db
-
+from conditional.util.auth import get_username


Move to style fixes commit.

liam-middlebrook · 2018-03-03T04:29:58Z

conditional/blueprints/spring_evals.py

 from conditional.models.models import HouseMeeting
+from conditional.models.models import MajorProject


Move to style fixes commit.

mbillow

Thanks for putting in this work! Just a few things I think could be moved around or fixed.

mbillow · 2018-03-03T23:13:56Z

conditional/util/auth.py

@@ -24,3 +26,13 @@ def wrapped_func(*args, **kwargs):
                     "is_eval": is_eval}, *args, **kwargs)

    return wrapped_func
+
+
+def get_username(func):


Just a thought: Almost every time we use this wrapper we immediately grab their LDAP object, why not just do that in the wrapper and return the user object?

mbillow · 2018-03-03T23:15:57Z

conditional/util/flask.py

-
-    # TODO maybe use the webauth request decorator
+@get_username
+def render_template(template_name, username=None, **kwargs):


See my previous comment. We should just include this in the wrapper and then we don't have to worry about it.

mbillow · 2018-03-03T23:16:44Z

config.sample.py

@@ -25,5 +26,15 @@
 SQLALCHEMY_DATABASE_URI = 'sqlite:///{}'.format(os.path.join(os.getcwd(), "data.db"))
 ZOO_DATABASE_URI = 'mysql+pymysql://user:pass@host/database'

+# OIDC Config


Please also add some relevant info to the README in the setup instruction section.

devinmatte · 2018-03-04T02:50:32Z

Addressed the comments left by @mbillow

Honestly, I don't think that a separate style commit is necessary. It doesn't net us any real benefits.

stevenmirabito

🚢 🎉

devinmatte added 2 commits March 2, 2018 21:39

Adding flask_pyoidc

d214bdf

Style Fixes

13d2fb6

liam-middlebrook previously requested changes Mar 3, 2018

View reviewed changes

Updating for Python 3.6

4a84fd2

devinmatte force-pushed the oidc branch from 25b015d to 4a84fd2 Compare March 3, 2018 23:14

mbillow requested changes Mar 3, 2018

View reviewed changes

devinmatte force-pushed the oidc branch 4 times, most recently from 42ef6df to 317cd95 Compare March 4, 2018 02:44

Adding Account to Wrapper and README Updated

30118bd

devinmatte force-pushed the oidc branch from bf35ac8 to 30118bd Compare March 4, 2018 05:46

devinmatte mentioned this pull request Mar 4, 2018

Minify HTML to Reduce Load time and Data #170

Open

mbillow approved these changes Mar 5, 2018

View reviewed changes

Fixing Requirements

da9a8fe

stevenmirabito approved these changes Mar 6, 2018

View reviewed changes

stevenmirabito merged commit 54c27fb into ComputerScienceHouse:develop Mar 6, 2018

		# pylint: disable=C0413
		from conditional.models.models import UserLog


		from conditional.blueprints.dashboard import dashboard_bp # pylint: disable=ungrouped-imports
		from conditional.blueprints.dashboard import dashboard_bp # pylint: disable=ungrouped-imports

		from flask import Blueprint, request, jsonify

		from conditional import db, auth

		from flask import Blueprint, request

		from conditional import auth


		from conditional import db

		from conditional.util.auth import get_username

		from conditional.models.models import HouseMeeting
		from conditional.models.models import MajorProject

Adding Flask_pyoidc and Updating to Python 3.6 #169

Adding Flask_pyoidc and Updating to Python 3.6 #169

Uh oh!

Conversation

devinmatte commented Mar 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liam-middlebrook left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devinmatte Mar 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbillow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devinmatte commented Mar 4, 2018

Uh oh!

stevenmirabito left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devinmatte commented Mar 3, 2018 •

edited

Loading

devinmatte Mar 3, 2018 •

edited

Loading