-
Notifications
You must be signed in to change notification settings - Fork 94
Support Data Sources #196
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
base: main
Are you sure you want to change the base?
Support Data Sources #196
Conversation
y-kawawa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wolveix Thanks for the PR! I’ve left a comment.
|
|
||
| type DatabaseService interface { | ||
| Create(ctx context.Context, request *DatabaseCreateRequest) (*Database, error) | ||
| Query(context.Context, DatabaseID, *DatabaseQueryRequest) (*DatabaseQueryResponse, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this depends on the official Notion fix, removing it counts as a breaking change.
It might be a good idea to update the module path to v2, in line with semantic versioning.
Alternatively, if you’d prefer not to move to v2, another option could be to provide two notionVersions - 2022-06-28 and 2025-09-03 - and support both.
Of course, that would increase maintenance costs, so it also depends on the policy of this repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment! From reading Notion's release, it doesn't sound like they'll be supporting database querying at all moving forward (as I understood it, even querying with the old API version will outright fail with new databases).
I think moving to v2 makes the most sense. Though in saying that, I do believe this library has similarly had breaking changes in the past (the last commit in the repo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the response.
I also feel that moving to v2 would be a good option, but if this repository has followed a different approach in the past, it might be better to keep going that way.
y-kawawa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wolveix Additional comment.
|
Does it get merged or should we fork it and maintain it ourselves? |
|
Thanks for bumping this @JanRuettinger! As a last ditch effort, I've just emailed the author (@jomei) asking whether they'd consider taking on additional maintainers (I just went through this same process with Huma). If I don't hear back soon, I'd be more than happy to switch the branches around on my repo and update package references accordingly :) I have a lot of great ideas for this library, especially around ease-of-use. I've built an entire wrapper library around this one to help simplify working with Notion webhooks (introduced last year) as well as ease model translation between native Go types and Notion models. Frankly I think these should all be part of the library itself. |
|
Sorry, not trying to close the PR, was just trying to rename the branch in my repo. Annoying. |
|
I've updated package references, so you can now replace I will likely also rename all known abbreviations/acronyms to block capitals, e.g. And I'd be happy to PR these back to this repo as and when @jomei has the time :) |
Hello!
We rely on this library for various automations and internal tools, so the announcement of Data Sources from Notion concerned me (with regards to when it may be implemented here).
This is my first time working directly with Notion's API (outside of webhooks), so my apologies if anything seems odd! I ran through the upgrade guide and implemented all of the changes I could see.
Do note that I haven't added all tests for data sources yet.
This closes #195.