-
Notifications
You must be signed in to change notification settings - Fork 130
feat: Document new synthetic default dataset item event #2007
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: master
Are you sure you want to change the base?
Conversation
Preview for this PR was built for commit |
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.
Looks good to me. Maybe we can also add some section on explaining why this is very good for moving scrapers from rental/PPR
to PPE
, as that's one of the cool things about this, that it'll make it super easy. I suggest to elaborate on this more.
| Commission opportunities| Standard 20% | Standard 20% | Promotional 0% periods (until 01/11/2025) | | ||
| Custom event billing | Not available | Not available | ✅ Charge for any event | | ||
| Per-result billing | Not available | ✅ Charge per dataset item | Optional (via event) | | ||
| Per-result billing | Not available | ✅ Charge per dataset item | Optional (via event; automatic via `apify-actor-default-dataset-item`) | |
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.
Q: how does the table look, isn't the last column super wide?
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.
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.
It should break nicely, we can also make use of <br/>
in the worst case scenario to force text into new line how we want it
|
||
#### How the synthetic default dataset item event works | ||
|
||
- Apify automatically charges this event whenever your Actor writes an item to the default dataset (for example, when using `Actor.pushData` without opening a named dataset). |
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.
Suggestion: without opening a named dataset
part is misleading. This only works for pushing into the default dataset.
| Commission opportunities| Standard 20% | Standard 20% | Promotional 0% periods (until 01/11/2025) | | ||
| Custom event billing | Not available | Not available | ✅ Charge for any event | | ||
| Per-result billing | Not available | ✅ Charge per dataset item | Optional (via event) | | ||
| Per-result billing | Not available | ✅ Charge per dataset item | Optional (via event; automatic via `apify-actor-default-dataset-item`) | |
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.
It should break nicely, we can also make use of <br/>
in the worst case scenario to force text into new line how we want it
### Use synthetic default dataset item event `apify-actor-default-dataset-item` | ||
|
||
::::info Automatic per-item charging | ||
|
||
The `apify-actor-default-dataset-item` synthetic event charges users for each item written to the run's default dataset. It lets you align PPE pricing with per-result use cases without adding charging code to your Actor. | ||
|
||
:::: |
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.
If this is the only text in the heading, do we really need an admonition?
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.
I agree with @TC-MO, I would skip the admonition
#### How the synthetic default dataset item event works | ||
|
||
- Apify automatically charges this event whenever your Actor writes an item to the default dataset (for example, when using `Actor.pushData` without opening a named dataset). | ||
- No code changes are required to charge this event. | ||
- You can remove the event in Apify Console if you don't want automatic charging for default dataset items. If you remove it, default dataset writes will no longer be charged automatically. | ||
- The event applies only to the default dataset of the run. Writes to other (non-default) datasets are not charged by this synthetic event. |
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.
I'm not a fan of bullet lists like this for whole content of the heading, if possible I would recommend something like:
#### How the synthetic default dataset item event works | |
- Apify automatically charges this event whenever your Actor writes an item to the default dataset (for example, when using `Actor.pushData` without opening a named dataset). | |
- No code changes are required to charge this event. | |
- You can remove the event in Apify Console if you don't want automatic charging for default dataset items. If you remove it, default dataset writes will no longer be charged automatically. | |
- The event applies only to the default dataset of the run. Writes to other (non-default) datasets are not charged by this synthetic event. | |
#### How the synthetic default dataset item event works | |
Apify automatically charges this event whenever your Actor writes an item to the default dataset, for example, when using `Actor.pushData()`. This automatic charging requires no code changes on your part. | |
The event applies only to the default dataset of the run. Writes to other (non-default) datasets are not charged by this synthetic event. | |
If you don't want automatic charging for default dataset items, you can remove the event in Apify Console. Once removed, default dataset writes will no longer be charged automatically. |
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.
We have the same formatting for How the synthetic start event works
, I suggest to keep it same and use bullet lists.
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.
If we do it once, it's fine, if it starts turning into a pattern, then it's not a good one and we should be avoiding it, thus my recommendation to change it.
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.
Ok, its minor thing, lets not block the PR by it.
I agree with @mhamas about explaining why this is very good for moving scrapers from rental/PPR to PPE. Otherwise it looks good, thanks. |
Preview for this PR was built for commit |
resolves: https://github.com/apify/apify-core/issues/23408