Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Upcoming Sprint #594

Closed
paltman opened this issue Sep 26, 2018 · 33 comments
Closed

Upcoming Sprint #594

paltman opened this issue Sep 26, 2018 · 33 comments

Comments

@paltman
Copy link
Contributor

paltman commented Sep 26, 2018

I have been very swamped over the past number months since the last release and there are a lot of things we need to get back to to keep this project alive and healthy. I'm planning an upcoming sprint (still don't have a date yet, but will likely occur over a (long) weekend in the next 4-6 weeks.

I wanted to open this issue so that I could get feedback from folks of what their favorite issues would be to see knocked out, preferably with associated PRs to consider/review. Please link to them in the comments with any supporting details as to why it should be considered priority knowing that I'll have only so much that I can do in a fixed window of time.

Obviously, the more help I can get with this in terms of good PRs versus just Issues, the more that can get done.

@blueyed @lukeburden @ticosax as some of the top-most recent contributors to the project in way of commits, I'd love your help in triaging items on this issue, if you have any time to spare.

@lukeburden
Copy link
Contributor

Excellent call @paltman.

I'd like to see added support for storing Dispute objects. I notice that @blueyed has #591 up which fixes the charge updated webhook, but I'd be curious to go a bit further and store some of the Dispute's info to allow for more complex tracking in-app.

@tyndyll
Copy link

tyndyll commented Sep 26, 2018

I'd like to see the Connect Custom accounts extended to more countries than US/CA. Happy to help by submitting a PR

@blueyed
Copy link
Contributor

blueyed commented Sep 26, 2018

Connect Custom accounts extended to more countries than US/CA

Why/where is that limited?

@lukeburden
Copy link
Contributor

There are some dynamic choice fields for countries and states - ideally, we could remove that, and derive everything from Stripe's CountrySpec objects.

Are you thinking of a major release, allowing us to break backward compat? If so, two things we should include in this sprint:

  • add a Payout model separate to Transfer to properly reflect Stripe's changes.
  • review all the API updates since the default API version in this lib, adjust to reflect the latest API version, update the default API version in this lib to the latest

@paltman
Copy link
Contributor Author

paltman commented Oct 8, 2018

I'd also like to see updates for what is needed for changes in Subscription model, naming the move Stripe made a while back to add Product objects that each have a Plan that you subscribe too. I think @blueyed's #588 cracks the door a bit to address this.

@paltman
Copy link
Contributor Author

paltman commented Oct 8, 2018

@lukeburden it might be easiest to have the freedom of a major release and then we can consider how to back port / maintain certain things but I'm wary of getting into any multiple release management. I can barely keep up with just a single release. :)

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

Does anyone see any problem with the next release being a major release that just pins to latest API version and updates everything to match the changes since last API version?

Trying to maintain BC is going to be a pain and not sure it's worth it, especially given that the models in this app are just storing a cache of what's in Stripe. It should always be safe to rehydrate your local cache from the API in what are essentially read only models. If that's not working this there is a bug that we should fix.

Thoughts?

@lukeburden
Copy link
Contributor

lukeburden commented Oct 9, 2018 via email

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

I've started the Next milestone. I'd like to keep it relatively focused though I don't mind loading it up with outstanding PRs that are non-controversial and easily forward ready.

My biggest goal for this next release is to get it all updated for latest API version.

@paltman paltman added this to the Next milestone Oct 9, 2018
@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

I've gone through the change log and I think these are the items that affect us:

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

crap, that's a lot.

@lukeburden
Copy link
Contributor

Divide and conquer!

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

@lukeburden there. they are now divided. :)

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

I've had a chance to play around with py.test a bit too since @blueyed first suggested we move to using it and I'm finally on board with that. Doesn't have to be apart of this milestone but if someone wants to update CI bits to run on py.text, it has my full support.

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

I wonder if we should add library identification.

Maybe it's as simple as adding:

stripe.set_app_info(
    "Pinax Stripe",
    version=PULLED_FROM_PKG_INFO,
    url="https://github.com/pinax/pinax-stripe"
)

blueyed added a commit to blueyed/pinax-stripe that referenced this issue Oct 9, 2018
@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2018

Migration to pytest is ready in #618.

I am not sure about API breakage / having to sync things manually really, but if it helps / is required to get the project back on track, sure.
Long-standing issues like #158 however still indicate that there are more general issues, where lacking manpower is missing.

@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2018

btw: what happened to the last sprint? https://github.com/pinax/pinax-stripe/milestone/12

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018 via email

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

@blueyed what do you mean by:

I am not sure about API breakage / having to sync things manually really, but if it helps / is required to get the project back on track, sure.

Do you think we can handle everything through data migrations?

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

where lacking manpower is missing.

@blueyed would you be willing/able to help me maintain this project?

@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2018

Do you think we can handle everything through data migrations?

I would expect so, yes.
But cannot really say if it is worth the effort.
For my work we're using a fork though anyway already (the "next" branch), and I would assume that it is OK for our work to stay with an older API version as long as Stripe supports it.

would you be willing/able to help me maintain this project?

Not really, sorry.

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

I would expect so, yes.
But cannot really say if it is worth the effort.

I think we can probably handle 90% of it fairly easily. But there are some parts that would require too much predicting what/how the person was doing with their stripe account. Did they create products? If so, we need to sync them down to get their IDs before we can create FKs, etc.

Not really, sorry.

No problem. It's a LOT of work and I understand everyone has priorities.

Stripe moves so quickly, I do feel like this project is moving beyond a single maintainer in order to keep up.

@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2018

Yeah - but it's also about not merging trivial thinks like #593 right away. I'd be happy to help out with this (i.e. not just approve them, but merge them when coming across those)

Anyway, good to see some activity here again.
Please also take a look at #558.

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

@blueyed happy to give you access to merge if you want to help with the low-hanging fruit ones like that. yes,. there are trivial ones like that, but it would be a HUGE help to me.

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

I honestly just miss a lot of them.

screen shot 2018-10-09 at 4 11 33 pm

@blueyed
Copy link
Contributor

blueyed commented Oct 9, 2018

@paltman
Sure, happy to help.
tip: you can start with "Participating".. ;)

@paltman
Copy link
Contributor Author

paltman commented Oct 9, 2018

Thanks. I think I mostly just need to carve out time each week to go through my Github messages/review/merge, separate and apart from my desire to code on things.

@paltman
Copy link
Contributor Author

paltman commented Oct 20, 2018

The more I have thought about this and the more I talk with some of my team members, the more I'm convinced the right strategy going forward for this app is to make it more of a "toolkit" than a complete cache/wrapper.

The sheer number of API methods and the rate at which new ones get added and the new versions of the API that are released, make it nearly impossible to maintain pace. Unless the Stripe team itself were to take over maintenance of this app, I don't see there being enough resources and man power, even with now two maintainers, to keep up.

In addition, Stripe's API is well documented and easy to understand. We have lacked solid documentation for a while in part because it's just a lot to do and to maintain so we never seem to get to it. It seems awkward to force the developer to have to know what they want to do on Stripe, see the Stripe docs and then have to figure out what that means in this package by picking the right action function.

I do think there is a lot of value in making it easy to capture and emit signals for web hooks out of the box. I also think there is value in shipping with some mixing and/or abstract models for the site building to construct whatever caching models are appropriate for their application.

Therefore, in a broad stroke, I think the next major version of this app should kill most models (I think we still need model(s) for the webhook events, at least), and all the actions. We can probably kill the views as well, as they are mostly CBV against the models we are getting rid of (and personally, I almost always override them).


With this in mind, I can't even begin to imagine what the upgrade path looks like, but maybe we sort that out once this new approach starts taking shape.

While this will make this app a lot leaner, I think we will be able to keep things up to date a lot better while still providing a lot of utility. We can then provide some "cookbook" or other type documentation for how to setup the caching models for certain common situations and perhaps open the door for secondary packages that depend on pinax-stripe to provide these caching models that can be maintained and released independently.

@blueyed what are you thoughts on this?

@lukeburden
Copy link
Contributor

@paltman that seems like a pragmatic and considered course of action, to me.

I'll have to think through how this would affect my projects that use this library, but I'm not using the cache models beyond the benefit of having something to point a foreign key at. Storing the stripe-id for the related object in a non-null field would probably be just about as effective.

The webhooks processing is an important utility in all my uses of this project.

There's a fair bit of complexity in their Connect product that the forms I contributed were intended to offset, but in practice, people could make these themselves or we could package them separately.

The upgrade path is probably a lot of work for most people, so I'm a bit worried we'll lose a lot of people on this move - but it has to be this way if you're certain it's infeasible to keep up with Stripe's pace of change.

@paltman
Copy link
Contributor Author

paltman commented Oct 22, 2018 via email

@lukeburden
Copy link
Contributor

@paltman I thought I'd check in with some further thoughts on this project.

I've recently been making some changes to a platform's payments system, and am increasingly using this library as almost solely a webhook processing utility. I typically store the Stripe ID in a unique field on a local model, and perform whatever behaviour is required on receipt of a webhook.

Often, if I need a bit more contextual data, I'll have a few fields on a local model. Often though, this model is not specific to Stripe, so this usually takes the form of a JSON field.

Basically, I still think your gut instincts are right for this project, and that as a stripped down set of utilities sans most models would have succinct and real value.

It's also worth noting that djstripe has continued to progress down the path of local model storage. They have a pretty strong group of project maintainers now, so they can seemingly keep up with the changes required. It would be the right thing to do after stripping back this project to link to them as an alternative that provides a full local DB cache.

@paltman
Copy link
Contributor Author

paltman commented Feb 19, 2019

@lukeburden great feedback. thanks! you comment also bumps this issue back into my view. I had completely forgotten about it.

@paltman paltman removed this from the Next milestone Nov 25, 2021
@paltman
Copy link
Contributor Author

paltman commented Nov 25, 2021

@lukeburden @blueyed @tyndyll it's been some years...

I've gotten way away from using the cache-model and service layer methods in my various projects and have just been using the webhooks on the 5.x branch so my plan is to cut a new v5 release after merging that open PR #620 (after a bit more work) and make this project just the lean bits that I'm finding useful. If someone is interesting in maintaining a fuller version based on the 4.x line we can split the project and I can make a pinax-stripe-light or something, but given that djstripe now appears to have robust commercial support and a team of active maintainers, it probably makes sense for people wanting/needing more to consider that project. Personally, I've found it overkill on projects that I'm active on that are doing significant volume in complex stripe connect based settings and therefore can't justify putting a lot time into maintenance.

@pinax pinax locked and limited conversation to collaborators Nov 26, 2021
@paltman paltman closed this as completed Nov 26, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

4 participants