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.

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

Add comment why not to use ~= clause in deps definition #154

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

vdusek
Copy link
Contributor

@vdusek vdusek commented Dec 5, 2023

Currently, we use the inclusive ordered comparison clause in our dependency requirements (except the apify packages).

I suggest playing safely by using a more strict compatible release clause as we use it for the dev dependencies or dependencies in the Apify Client.

I install the dependencies using make install-dev, then use pip freeze and copy the package versions it installed - the current versions we use.

@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Dec 5, 2023
@vdusek vdusek requested a review from fnesveda December 5, 2023 14:50
@vdusek vdusek self-assigned this Dec 5, 2023
@github-actions github-actions bot added this to the 78th sprint - Tooling team milestone Dec 5, 2023
@vdusek vdusek force-pushed the deps-uses-compatible-release-clause branch from 8dd93bd to e64573c Compare December 5, 2023 14:58
Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

We were using the compatible clause earlier, but we had to switch to the ordered clause because the compatible clause was too strict, and people couldn't install the SDK with some other libraries because their requirement specifiers were incompatible (even though the actual libraries were compatible). I think specifically it was about people not being able to use some newer version of httpx or pyee, I'm not sure now.

Anyway, I know we're risking that some of our dependencies will release some major breaking change and the SDK will break because of that, but I would say the risk is relatively small, and the annoyance to users would be bigger if we used the compatible clause. Good writeup about the tradeoffs is here.

I wish Python allowed you to install transitive dependencies in separate versions, same as Node does with node_modules, that would solve this issue.

@vdusek
Copy link
Contributor Author

vdusek commented Dec 5, 2023

@fnesveda Oh, thank you, that makes sense...

I'll close the PR without merging.

What do you think about relaxing the dependencies for the apify-client as well (here)? Although, there is only HTTPX.

Edit: I reverted the changes and just added an explanatory note.

@vdusek vdusek changed the title Use compatible release clause in deps definition Add comment why not to use compatible release clause in deps definition Dec 5, 2023
@vdusek vdusek changed the title Add comment why not to use compatible release clause in deps definition Add comment why not to use ~= clause in deps definition Dec 5, 2023
@vdusek vdusek requested a review from fnesveda December 5, 2023 15:43
Copy link
Member

@fnesveda fnesveda left a comment

Choose a reason for hiding this comment

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

Cool 👍 I would probably use the ordered clause in the client as well (and for the scrapy extra here too).

@vdusek vdusek merged commit cbed2b7 into master Dec 6, 2023
22 checks passed
@vdusek vdusek deleted the deps-uses-compatible-release-clause branch December 6, 2023 10:12
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. 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.

2 participants