Skip to content

Conversation

@vdusek
Copy link
Contributor

@vdusek vdusek commented Jan 22, 2024

Issue

Description

  • This PR moves just a single function from a template to SDK - apply_apify_settings.
  • The rest of the suggested changes are related to the logging. I was not able to make them work at first try. So for now let's solve it just for the settings-related helper function, and the rest will be done later.

Testing

  • The changes were tested both locally and on the platform.
  • Unit tests are ready as well.

@vdusek vdusek changed the title Add apply_apify_settings Add apply_apify_settings to Scrapy subpackage Jan 22, 2024
@vdusek vdusek added t-tooling Issues with this label are in the ownership of the tooling team. adhoc Ad-hoc unplanned task added during the sprint. enhancement New feature or request. labels Jan 22, 2024
@vdusek vdusek added this to the 81st sprint - Tooling team milestone Jan 22, 2024
@vdusek
Copy link
Contributor Author

vdusek commented Jan 22, 2024

@honzajavorek FYI; also if you would have any feedback on this, I'll be glad. Thanks 🙂.

Copy link
Contributor

@honzajavorek honzajavorek left a comment

Choose a reason for hiding this comment

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

The implementation looks good, awesome! I've found a few places in the tests where I commented, but they're nothing serious except for the @pytest.mark.only() leftover, which was the decisive factor in why I'm requesting changes.

@vdusek vdusek requested a review from honzajavorek January 23, 2024 09:50
Copy link
Contributor

@janbuchar janbuchar left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@vdusek
Copy link
Contributor Author

vdusek commented Jan 23, 2024

Thank you both guys @honzajavorek, @janbuchar for your opinions. Merging it.

@vdusek vdusek merged commit 72a37f1 into master Jan 23, 2024
@vdusek vdusek deleted the improve-scrapy-integration branch January 23, 2024 12:40
@vdusek vdusek self-assigned this Jan 23, 2024
@honzajavorek
Copy link
Contributor

Thanks for shipping it! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. enhancement New feature or request. t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants