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.

Sign up for GitHub

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

Jump to bottom

docs: add resourceType property to support storage picker #1261

Merged
merged 6 commits into from
Nov 14, 2024
Merged

docs: add resourceType property to support storage picker #1261

merged 6 commits into from
Nov 14, 2024

Conversation

MFori
Copy link
Contributor

@MFori MFori commented Oct 22, 2024

Proposal of new input schema property resourceType (based on https://apify.slack.com/archives/C01VBUV81UZ/p1729081102142439?thread_ts=1728031818.871689&cid=C01VBUV81UZ) which will solve the needs of storages picker (https://github.com/apify/apify-core/issues/12871)

As I already verified, in ajv schema there would be two new top level properties resourceProperty and resourceArrayProperty, ajv can distinguish between these and stringProperty with arrayProperty based on the required resourceType property. The editor property is optional with default value of resourcePicker.
Input schema PR will come when this one is approved.

The description is tightly connected with storages even though later the resourceType should support other types of resources, but for now I think it's ok like this.

Preview of docs:

Screenshot 2024-10-22 at 19 47 38 Screenshot 2024-10-22 at 19 47 51

@MFori MFori added documentation Improvements or additions to documentation. t-console Issues with this label are in the ownership of the console team. labels Oct 22, 2024
@MFori MFori self-assigned this Oct 22, 2024
@MFori MFori requested a review from TC-MO as a code owner October 22, 2024 17:58
@github-actions github-actions bot added this to the 101st sprint - Console team milestone Oct 22, 2024
jancurn
jancurn reviewed Oct 23, 2024
View reviewed changes
sources/platform/actors/development/actor_definition/input_schema/specification.md Outdated
@@ -409,3 +409,41 @@ Editor type `select` allows the user to pick items from a select, providing mult
```

To correctly define options for multiselect, you need to define the `items` property and then provide values and (optionally) labels in `enum` and `enumTitles` properties.

### Resource
Copy link
Member

Choose a reason for hiding this comment

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

Why not "Resource type" ?

sources/platform/actors/development/actor_definition/input_schema/specification.md Outdated

### Resource

The Resource property enables Actor users to select different types of resources.
Copy link
Member

Choose a reason for hiding this comment

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

I'd say that:

Resource type identifies what kind of object is referred to in the input field. For example, a the Key-value store resource type can be referred to using a string ID.

mtrunkat
mtrunkat requested changes Oct 23, 2024
View reviewed changes
Copy link
Member

@mtrunkat mtrunkat left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I am missing info what is the returned value in a case of an array vs string. There is screenshot only for string. I'd also improve the wording there mentioning that the return value is the resource ID itself.

Otherwise good to go after the other comments are fixed.

@MFori
Copy link
Contributor Author

MFori commented Oct 23, 2024

I am missing info what is the returned value in a case of an array vs string. There is screenshot only for string. I'd also improve the wording there mentioning that the return value is the resource ID itself.

Otherwise good to go after the other comments are fixed.

I update it based on @jancurn comments and try to better mention that the return value is the resource ID:
I can also include screenshot for array input once we have it implemented.

Screenshot 2024-10-23 at 12 05 23

gippy
gippy approved these changes Oct 23, 2024
View reviewed changes
Copy link
Member

@gippy gippy left a comment

Choose a reason for hiding this comment

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

Just one question. Since these docs already mention, that we support arrays too. Are you planning to update the resource picker in UI to support arrays?

If not then we should not mention arrays until we actually decide that we want to implement them.

@MFori
Copy link
Contributor Author

MFori commented Oct 23, 2024

Just one question. Since these docs already mention, that we support arrays too. Are you planning to update the resource picker in UI to support arrays?

If not then we should not mention arrays until we actually decide that we want to implement them.

Yeah I have almost done it in UI too

@jancurn
Copy link
Member

jancurn commented Oct 24, 2024

I assume we don't allowed both string and array at the same time, that the input type can be just one of them. Right?

@MFori
Copy link
Contributor Author

MFori commented Oct 24, 2024

I assume we don't allowed both string and array at the same time, that the input type can be just one of them. Right?

Exactly, there is still the type property with value of string or array:

Screenshot 2024-10-24 at 8 17 34

netmilk
netmilk approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@netmilk netmilk left a comment

Choose a reason for hiding this comment

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

It looks all ok to me!

One small thing to consider: adding context to the terms "object" and "resource" where possible makes it easier to understand they are related to Apify's architecture and not just a "resource" or "object" from the generic programming or integration/API domain language.

sources/platform/actors/development/actor_definition/input_schema/specification.md Outdated Show resolved Hide resolved
sources/platform/actors/development/actor_definition/input_schema/specification.md Outdated Show resolved Hide resolved
@MFori MFori mentioned this pull request Oct 24, 2024
Merged
MFori and others added 3 commits October 25, 2024 00:04
@MFori @netmilk
Update sources/platform/actors/development/actor_definition/input_sch…
cd3e685 
…ema/specification.md

Co-authored-by: Adam Kliment <79609+netmilk@users.noreply.github.com>
@MFori @netmilk
Update sources/platform/actors/development/actor_definition/input_sch…
91fb482 
…ema/specification.md

Co-authored-by: Adam Kliment <79609+netmilk@users.noreply.github.com>
@MFori
fix typos
9f18a10
MFori added a commit to apify/apify-shared-js that referenced this pull request Oct 30, 2024
@MFori
feat(input_schema): Resource properties (#484)
ccfca82 
Update of input schema based on
apify/apify-docs#1261:
New properties in schema: `resourceProperty` and `resourceArrayProperty`
To distinguish between resource properties with `resourceType` and
string/array properties custom validation is done in `validateField`
(because of clash on same `type`)

Input ui to support this schmena is ready here:
apify/apify-core#18093
@MFori MFori merged commit 97830de into master Nov 14, 2024
7 checks passed
@MFori MFori deleted the feat/resource-type-input-property branch November 14, 2024 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jancurn jancurn jancurn approved these changes

@gippy gippy gippy approved these changes

@mtrunkat mtrunkat mtrunkat approved these changes

@netmilk netmilk netmilk approved these changes

@TC-MO TC-MO Awaiting requested review from TC-MO TC-MO is a code owner

Assignees

@MFori MFori

Labels
documentation Improvements or additions to documentation. t-console Issues with this label are in the ownership of the console team.
Projects
None yet
Milestone
101st sprint - Console team
Development

Successfully merging this pull request may close these issues.
5 participants
@MFori @jancurn @netmilk @mtrunkat @gippy