Skip to content

Conversation

@benmccann
Copy link
Contributor

@benmccann benmccann commented Sep 11, 2025

Closes #310

This is a breaking change as it switches from positional arguments to a single argument with property-based parameters.

The provider interface is quite simple and I don't expect it would need to be extended in the future. Luckily arctic's design is quite nice where most of the per-provider custom logic is hidden away internally in the provider or requested through the constructor, so these common methods are quite standard. However, if the need ever arose, it could be extended to support some new provider field without a breaking change for the existing providers. If there were one that was really off the walls you could still implement a custom class without implementing the interface, but again I don't forsee that as ever being necessary.

If we wanted to avoid breaking existing users and make for an easier migration path we could leave the existing methods with positional arguments in place alongside the new methods and could simply have one call the other.

I put OAuth2 in the class names, but am not 100% sure that's correct or if arctic could possibly be used with OAuth 1 as well, so am open to renaming those.

I didn't update the docs yet as I wanted feedback on this change before doing so, but would be happy to update the docs if you're amenable to this change.


DO NOT DELETE THIS SECTION.

Thank you for creating a pull request!

If your pull request is just making changes to the docs, please create it against the main branch.

If your pull request is making changes to the library source code, please create it against the next branch. If your pull request adds a new feature to the library, please open a new issue first.

If you're unsure, you can just create it against the main branch.

  • Please tick this box if you’ve read and understood this section..

@benmccann
Copy link
Contributor Author

@pilcrowonpaper sorry to do this, but any chance I might be able to ask for a review on this?

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.

1 participant