feat: add `abort_on_error` property by Mantisus · Pull Request #731

feat: add `abort_on_error` property by Mantisus · Pull Request #731 · apify/crawlee-python

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'),

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 left a comment •

Loading

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.)

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

Choose a reason for hiding this comment

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

LGTM

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdusek left a comment • Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vdusek left a comment •

Loading