Skip to content

feat: add abort_on_error property#731

Merged
janbuchar merged 2 commits into
apify:masterfrom
Mantisus:add-fail-fast
Nov 25, 2024
Merged

feat: add abort_on_error property#731
janbuchar merged 2 commits into
apify:masterfrom
Mantisus:add-fail-fast

Conversation

@Mantisus

@Mantisus Mantisus commented Nov 22, 2024

Copy link
Copy Markdown
Collaborator

Description

  • Implement abort_on_error aborting execution when an error occurs

Issues

Testing

  • Implemented a new unit test

Checklist

  • CI passed

@janbuchar janbuchar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, but the naming in tests could be improved



@pytest.mark.parametrize(
('n_requests', 'n_error_request', 'e_starts', 'e_finished'),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you rename these so that it's more obvious what they do? The seconds one is the most hard one to understand for me.

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good! However, considering this is a new user-facing option, I would like to know the opinion from @B4nan on the name (abort_on_error) as well, what do you think? (I'm ok with it.)

@B4nan

B4nan commented Nov 25, 2024

Copy link
Copy Markdown
Member

Sounds good to me, I liked the fail_fast too, but abort_on_error is probably easier to understand.

@vdusek vdusek left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@janbuchar janbuchar merged commit 6dae03a into apify:master Nov 25, 2024
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.

Implement a fail_fast flag for automatic run failures on errors

4 participants