Skip to content
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

Ag/notion source update #242

Merged
merged 6 commits into from
Aug 15, 2023
Merged

Ag/notion source update #242

merged 6 commits into from
Aug 15, 2023

Conversation

dat-a-man
Copy link
Collaborator

@dat-a-man dat-a-man commented Aug 14, 2023

Tell us what you do here

  • implementing verified source (please link a relevant issue labelled as verified source)
  • fixing a bug (please link a relevant bug report)
  • improving, documenting, customizing existing source (please link an issue or describe below)
  • anything else (please link an issue or describe below)

Relevant issue

issue # 241

More PR info

This PR addresses the issue where the Notion pipeline expects both id and use_name when fetching data from custom databases. Previously, omitting use_name would lead to a KeyError.

Changes made:

If use_name is not provided, the pipeline will now automatically retrieve it from Notion. For example, if the input is:

selected_database_ids = [{"id": "0517dae9"},{"id":"d8ee2d159a"}]
# The pipeline will fetch the corresponding use_name directly from Notion.

This not only prevents the aforementioned error but also improves the user experience, making the pipeline a bit more user-friendly and intuitive.

Users can also give input as:

selected_database_ids = [{"id": "0517dae9", "use_name":"name1"},{"id":"d8ee2d159a"}]
# the pipeline takes the first use_name from the input provided and fetches the second one automatically. 

Please review and let me know if there are any additional changes needed.

@@ -151,3 +151,14 @@ def search(
next_cursor = response.get("next_cursor")
has_more = next_cursor is not None
start_cursor = next_cursor

def get_database(self, database_id: str) -> Dict[str, Any]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linter failed. change Dict[str, Any] to Any.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rest looks good, well done))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't forget to edit docstring

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes done. Please see.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR passed the checks @AstrakhantsevaAA 👍 please review. Thanks

@AstrakhantsevaAA AstrakhantsevaAA merged commit 3ed20fc into master Aug 15, 2023
8 checks passed
@AstrakhantsevaAA AstrakhantsevaAA deleted the ag/notion_source_update branch August 15, 2023 12:39
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.

2 participants