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

Support adding a row from a template #266

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ITAYC0HEN
Copy link
Contributor

Hey 👋
This pull request implements a naive way to add a new row to a collection where its origin is a template (another block_id).

The implementation is based on a sequence of API calls:

  • Enqueue a new task of type duplicateBlock (API: v3/enqueueTask)
  • Loop N times every X seconds (or milliseconds) and check the status of the task (API: v3/getTasks)

The usage is very straightforward:

cv = client.get_collection_view("...")
cv.collection.add_row(sourceBlockId="75dd203a-EXAMPLE-a12b-ba492c37a1f6")

And of course, more parameters can be added:

cv.collection.add_row(
    sourceBlockId="75dd203a-EXAMPLE-a12b-ba492c37a1f6",
    name="The Hen House: Episode #123",
    type="Podcast"
)

Note:
The v3/getTasks actually supports getting the status of multiple tasks. I used it to get the status of a single one. We can consider improving it to support multiple tasks but didn't find it useful in the current design.

closes #194

Copy link
Owner

@jamalex jamalex left a comment

Choose a reason for hiding this comment

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

This is awesome! Very useful.

I agree that longer-term it would be nice to have a more general interface for task management, but unless we were going to make things pretty async, the current approach seems great.

A couple small suggestions.

notion/collection.py Outdated Show resolved Hide resolved
notion/client.py Outdated

return None

def wait_for_task(self, task_id, interval=1, tries=5):
Copy link
Owner

Choose a reason for hiding this comment

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

Would this be long enough for a huge template doc (with lots of children)? Worth upping the tries to 10, do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.
We can also consider passing interval and tries as an optional variable to add_row(...) so it can be configured when calling the function that uses wait_for_task(...) and not only when directly calling wait_for_task(...).

So currently, users can't control the intervals and tries of add_row(...) and I am not sure they should

@ag0n1k
Copy link
Contributor

ag0n1k commented Feb 7, 2021

Hi there!

My next step is use template on creation, did some CR, if u don't mind. ;)

Copy link
Contributor

@ag0n1k ag0n1k left a comment

Choose a reason for hiding this comment

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

Sorry for the duplication, it's all new for me)

Comment on lines +382 to +387
data = self.post(
"getTasks",
{
"taskIds": [task_id]
}
).json()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
data = self.post(
"getTasks",
{
"taskIds": [task_id]
}
).json()
data = self.post("getTasks", dict(taskIds=[task_id]).json()

if results is None:
return None

if not results:
Copy link
Contributor

Choose a reason for hiding this comment

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

That is the same with

if results is None:

Or something I don't know =)

elif state == "success":
return state

print("Task takes more time than expected. Specify 'interval' or 'tries' to wait more.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
print("Task takes more time than expected. Specify 'interval' or 'tries' to wait more.")
logger.debug("Task takes more time than expected. Specify 'interval' or 'tries' to wait more.")

# Start a duplication task
data = self._client.post(
"enqueueTask",
{
Copy link
Contributor

Choose a reason for hiding this comment

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

i would rather use dict() instead of {}, especially when the keys are constants (not variables). But this decision up to @jamalex

Comment on lines +398 to +402
if len(results) == 1:
state = results[0].get("state")
return state

return None
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if len(results) == 1:
state = results[0].get("state")
return state
return None
try:
task = results.pop()
return task.get("state", None)
except IndexError:
logger.error("There is no task {}".format(results))
return None

yashgorana added a commit to yashgorana/notion-py that referenced this pull request Aug 26, 2021
- Change is a fork of jamalex#266
  with some changes
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.

Is it possible to create a new row from a page template in a database?
3 participants