-
Notifications
You must be signed in to change notification settings - Fork 82
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
feat: add Actor Standby documentation #1086
Conversation
mhamas
commented
Jun 27, 2024
•
edited
Loading
edited
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, I left a few comments.
@jirimoravcik since Maťo is on vacation, can you check my suggestions, and if you're fine with them, just commit them?
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.
This should be probably checked by some native speaker 😅
Also not sure that mode has "the" article. 🤔
Content-wise ok, just the comments that are already there + nits
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, this needs to wait for public release 😅 Working on it, blocked by https://github.com/apify/apify-core/pull/16488
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.
Current screenshots can be taken from
https://console-featstandbypublicrelease-a00865.apify.com/
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.
This will look a bit different, currently in PR
inputs and ouputs in your README. | ||
|
||
#### Can I monetize my Actor in the Standby mode? | ||
No, the Standby mode is currently in Beta and the monetization is not supported. |
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.
[To consider]: I am thinking that some readme example would be nice, with #actor-standby header
Co-authored-by: František Nesveda <[email protected]> Co-authored-by: Zuzana Štětinová <[email protected]>
Co-authored-by: TheoVasilis <[email protected]>
fix mdlint issues remove unnecessary words slight changes for clarity
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.
LGTM
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, except that we need to add a section about how to actually develop the Standby Actors, and some example Actor etc.
sources/platform/actors/running/images/actor_standby/standby-tab.png
Outdated
Show resolved
Hide resolved
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, I left a few suggestions
|
||
You can head to the Settings tab of your Actor, enable Standby mode, and set the default configuration. | ||
![Standby for creators](./images/actor_standby/standby-creators.png) | ||
|
||
For the coding part, you can use Apify SDK to retrieve the port Actor Standby should be listening on. |
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.
Can you talk here a bit more about how the Actors should behave internally before jumping straight to the code?
Sth like "Standby Actors should run a HTTP server listening on a specific port, to which the user requests will be proxied. This port is available in the Actor config object available in the Apify SDK."
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.
Done
<Tabs> | ||
<TabItem value="JavaScript" label="JavaScript"> | ||
|
||
```js | ||
import { Actor } from 'apify'; | ||
|
||
await Actor.init(); | ||
console.log(Actor.config.get('standbyPort')); | ||
``` | ||
|
||
</TabItem> | ||
<TabItem value="Python" label="Python"> | ||
|
||
```python | ||
from apify import Actor, Configuration | ||
|
||
async def main() -> None: | ||
async with Actor: | ||
config = Configuration.get_global_configuration() | ||
print(config.standby_port) | ||
``` | ||
|
||
</TabItem> | ||
</Tabs> |
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 would skip this and go straight for the example with the HTTP server, it's not too complicated, so there's no need to split it into two examples.
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.
Done
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, just a few last suggestions and then we're good to go
Co-authored-by: František Nesveda <[email protected]>
Co-authored-by: Jiří Moravčík <[email protected]> Co-authored-by: František Nesveda <[email protected]> Co-authored-by: Zuzana Štětinová <[email protected]> Co-authored-by: TheoVasilis <[email protected]> Co-authored-by: Michał Olender <[email protected]>