Skip to content

Add actor-related and run-related resource & resource collection clients - #37

Merged
fnesveda merged 5 commits into
masterfrom
feature/actors-and-runs-clients
May 13, 2021
Merged

Add actor-related and run-related resource & resource collection clients#37
fnesveda merged 5 commits into
masterfrom
feature/actors-and-runs-clients

Conversation

@fnesveda

@fnesveda fnesveda commented Apr 7, 2021

Copy link
Copy Markdown
Member

This PR:

  • adds the clients for actors, actor versions, runs & their respective collections
  • integrates the run-related additions to the placeholders in the task client
  • refactors the key-value store record encoding code out of the key-value store client, as it's used in more places now (for run inputs, since those are just key-value store records in the end)
  • slightly reorders the clients in docs to group them a bit more logically (this could still use more thought later)

I've tried to test this as much as I could, but there are honestly so many combinations of inputs to the methods that I'm afraid I missed some, and I apologize in advance if there are some bugs. I can't wait for when we implement proper integration tests.

@fnesveda
fnesveda requested review from Equidem and mhamas April 7, 2021 22:48
@fnesveda fnesveda self-assigned this Apr 7, 2021
@fnesveda fnesveda added the enhancement New feature or request. label Apr 7, 2021
@fnesveda fnesveda added this to the 10th Sprint - Platform team milestone Apr 7, 2021
This was linked to issues Apr 7, 2021
Implement actor-related resource clients #5
Closed
Implement run-related resource clients #11
Closed
@mhamas

mhamas commented Apr 8, 2021

Copy link
Copy Markdown
Contributor

@fnesveda is this ready for review? It still has draft label. thx

@fnesveda
fnesveda marked this pull request as ready for review April 8, 2021 06:51
@fnesveda

fnesveda commented Apr 8, 2021

Copy link
Copy Markdown
Member Author

@mhamas Oops, sorry, forgot to mark it ready, good catch :)

Equidem
Equidem suggested changes Apr 9, 2021
View reviewed changes
Comment thread src/apify_client/_utils.py Resolved
Comment thread src/apify_client/client.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor_version.py Resolved
Comment thread src/apify_client/clients/resource_clients/run.py Resolved
Base automatically changed from feature/misc-consolidation to master April 19, 2021 12:43
@fnesveda
fnesveda force-pushed the feature/actors-and-runs-clients branch from 46f5e17 to 757ff75 Compare April 19, 2021 12:57
@fnesveda
fnesveda requested a review from Equidem April 19, 2021 17:17
mhamas
mhamas requested changes Apr 20, 2021
View reviewed changes

@mhamas mhamas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What a beast, good job @fnesveda!

It looks mostly good to me, please check mainly the comment around using a dictionary instead of manually setting many fields, this happens in several functions.

Comment thread src/apify_client/_utils.py Resolved
Comment thread src/apify_client/clients/base/base_client.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor.py Resolved
Comment thread src/apify_client/clients/resource_clients/actor_collection.py Resolved
@fnesveda
fnesveda requested a review from mhamas April 20, 2021 13:40
@fnesveda
fnesveda merged commit 2e156d1 into master May 13, 2021
@fnesveda
fnesveda deleted the feature/actors-and-runs-clients branch May 13, 2021 11:22
@fnesveda fnesveda added the validated Issues that are resolved and their solutions fulfill the acceptance criteria. label May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Equidem Equidem Awaiting requested review from Equidem
@mhamas mhamas Awaiting requested review from mhamas

Assignees

@fnesveda fnesveda

Labels

enhancement New feature or request. validated Issues that are resolved and their solutions fulfill the acceptance criteria.

Projects

None yet

Milestone

10th Sprint - Platform team

Development

Successfully merging this pull request may close these issues.

Implement run-related resource clients Implement actor-related resource clients

3 participants

@fnesveda @mhamas @Equidem