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

fix(deps): Remove absolute requirements #331

Closed
wants to merge 5 commits into from

Conversation

Spacerat
Copy link
Contributor

Summary of Changes

This removes absolute requirements from the requirements.txt file.

Motivation

The specific requirement which has been causing me problems is attrs==18.1.0. I have been working on a Looker integration, but the looker_sdk library users a feature of attrs only available in >=19. When I install both libraries at the same time, pip installs attrs==18.1.0 due to amundsendatabuilder's restriction, and then looker_sdk fails.

It is worth noting that Looker ought to update their own setup.py to include attrs>=19.0.0 or something like that, but all that would do is make this problem happen at pip install time instead of runtime.

In general, pinning dependencies in libraries is liable to cause this kind of problem. I also noticed that a lot of the pinned dependencies were transitive dependencies not actually required by amundsendatabuilder itself. My feeling is that unless there's a specific reason to pin them due to e.g. a bug or issue, pip can sort these out itself.

Thus, I have removed all transitive dependencies in requirements.txt, and changed remaining dependencies from == to >=.

I have also incremented the version number to 3.1.0 so that this change can be picked up immediately.

This is a fairly significant change so I understand if it needs some work - please let me know if there's anything else I need to do.

Tests

No new tests needed. Hopefully this will just pass in CI.

Documentation

N/A

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
  • PR passes make test

Joseph Atkins-Turkish added 2 commits August 12, 2020 11:15
Signed-off-by: Joseph Atkins-Turkish <jatkins-turkish@brex.com>
Signed-off-by: Joseph Atkins-Turkish <jatkins-turkish@brex.com>
Signed-off-by: Joseph Atkins-Turkish <jatkins-turkish@brex.com>
@Spacerat Spacerat changed the title Remove absolute requirements fix(deps): Remove absolute requirements Aug 12, 2020
Joseph Atkins-Turkish added 2 commits August 12, 2020 11:57
Signed-off-by: Joseph Atkins-Turkish <jatkins-turkish@brex.com>
Signed-off-by: Joseph Atkins-Turkish <jatkins-turkish@brex.com>
Spacerat pushed a commit to brexhq/amundsendatabuilder that referenced this pull request Aug 12, 2020
This fixes the issue with the attrs transitive dependency described in amundsen-io#331 , without touching everything else.

Signed-off-by: Joseph Atkins-Turkish <jatkins-turkish@brex.com>
@Spacerat Spacerat mentioned this pull request Aug 12, 2020
5 tasks
feng-tao pushed a commit that referenced this pull request Aug 13, 2020
This fixes the issue with the attrs transitive dependency described in #331 , without touching everything else.

Signed-off-by: Joseph Atkins-Turkish <jatkins-turkish@brex.com>
@stale
Copy link

stale bot commented Aug 28, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale stalebot believes this issue/PR is no longer active label Aug 28, 2020
@stale
Copy link

stale bot commented Sep 19, 2020

This issue has been automatically closed for inactivity. If you still wish to make these changes, please open a new pull request or reopen this one.

@stale stale bot closed this Sep 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale stalebot believes this issue/PR is no longer active
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant