Refactor Django ORM

[waprin]

I was hoping to refactor some of django_orm into django_util. This also ties into #319. There are currently 3 classes:

1. CredentialsField - It makes sense to me for this to exist if you want to store the oauth credential as part of some sort of UserModel. You sometimes interact with the credential directly, and you might want to use it to do an oauth call in a background process outside of the session context, for example, in which case you want a credential in the database rather then only in a session.

2. Storage - This is a storage class that uses the CredentialField as the storage object. This will be one of the Storage options for our helpers to use.

The one that I think we should maybe get rid of is

3. FlowField - Back when people had to roll their own auth flow, this would be one way to store the Flow between the user being redirected to the authorization server and the callback being called with the code to obtain the credential. But now we are providing the code to do do that for you, and using the session seems pretty reasonable given the entire flow should happen within the context of a single session. The only counter-example I can think of is if you are trying to use cookie-based sessions and can't fit the credential into the session (this is currently biting us for our Flask example app, but I think the solution is just to use server-side sessions). Even if we want to keep it, we should probably just make it a configurable option for our code to use the ORM to store the flow rather then the session.

My inclination is just to remove FlowField but I'm not sure if people use it. I guess it comes down to whether people are still rolling their own flows and if we need to still support those use cases, even though we're now providing the view logic to do it for them.

What do people think?

[dhermes]

I am all for removing code. That entire file seems ill-conceived. I had a hard time figuring out many of the design decisions when I tried to test it a while back.

[theacodes]

@waprin can we close this in favor of the new plan for Django stuff?

[waprin]

This is basically the tracking issue for that work, although the scope of changes is somewhat bigger then just killing that field.

[nathanielmanistaatgoogle]

Retitle this issue to better summarize the intended improvements?

[waprin]

Retitled, basic plan is:

• Create usage docs to explain the two main use cases (oauth2 (Google) based user system vs adding oauth2 credentials into existing Django user system.
• Kill FlowField, Move CredentialFiield into django_util/models.py
• Keep existing decorator usage exactly as is, add a new config option that lets you specify a Model with a Django UserFIeld and CredentialField, and make the decorator store credentials in that field (where request.user matches the model's user)
• Figure out best place to add complete canonical samples for both main use cases, either directly in this repo or more likely linked to

[harel]

I know this is closed and I'm late to the game, but I'm not sure this was thought through. I can only relate my experience, not that it matters as its too late but still might be helpful:

• Django migrations that used the FlowField now error out. Apps won't even start until those older migrations are updated to be updated or removed. This kind of defeats the purpose of having those migrations because they were meant to rebuild the database change after change.
• My application does not use sessions. The back end is API based, using tokens, and there is no real guarantee two subsequent requests will hit the same physical machine. The FlowField in my case was vital to store the flow between oAuth steps. I'm sure this practice is not unique to me. I'm not that special :)
• Samples and docs are almost non existent - None offer a migration/upgrade path. This would have helped. The Google documentation still refers to FlowField and basically 2.x library constructs.

Many thanks regardless. I'll figure all this out - just thought its good to mention.

[waprin]

@harel thanks for feedback, some of it could have been better. Seemed like it needed updating, I tried to gather feedback on how it was being used but didn't get much. Now your explanation of how you're using things makes sense, I shouldn't have killed FlowField.

There are docs and samples in this repo's docs.

Apologies if it caused you any pain.

[harel]

Tha'ts fine @waprin. For posterity, what i did was simple enough and can be replicated by anyone:

• I migrated the FlowField to a Text field
• Then, I manually edited the original migration for the model that stored my FlowField to be a Text field. This stopped the new migration pre checks from failing.
• I added the pickle code lifted from the original FlowField into my model formerly known as the "model that contained the FlowField"
• Changed all refs to the django_orm pacakge, removed FlowField from imports and change Storage to the new DjangoORMStorage.
• I sat back, enjoyed the sunset and congratulated myself on a job well done while sipping gin and juice.

All is good - complaining about OS work done for free by good people is a cardinal ethics sin. So yes - its all good.

[qcaron]

Thank you @harel , I just got the code from release 2.2.0, grabbing the FlowField did the job it for me.