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.

Sign up for GitHub

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

Jump to bottom

Remove dependency on responses #1762

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Remove dependency on responses #1762

merged 1 commit into from
Jul 26, 2023

Conversation

honnix
Copy link
Contributor

@honnix honnix commented Jul 26, 2023

TL;DR

Remove dependency on responses

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

As reported in flyteorg/flyte#3895, there is no need to depend on responses package.

Tracking Issue

flyteorg/flyte#3895

Follow-up issue

Closes flyteorg/flyte#3895

@honnix
Remove dependency on responses
42d8c3a 
Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
@honnix
Copy link
Contributor Author

honnix commented Jul 26, 2023 •
edited

I was not able to rebuild all the requirements file for plugins because I got this type of error for each every plugin:

  The conflict is caused by:
      The user requested flytekitplugins-dask 0.0.0+develop (from /some/full/path/to/flytekit-dask)
      The user requested flytekitplugins-dask 0.0.0+develop (from .)

In the requirements.in file, I'm not sure why there is both . and the egg. On the same note, pip-compile now supports sourcing from setup.py so there is no need to have the requirements.in hack.

Even I could successfully do that for plugins, that would be a separate follow-up PR because this needs to be released first.

@honnix
Copy link
Contributor Author

honnix commented Jul 26, 2023

Another thing to note is that I was not able to do make doc-requirements.txt on Apple silicon mac because of pip._internal.exceptions.DistributionNotFound: No matching distribution found for tensorflow==2.8.1. I ended up with manually editing doc-requirements.txt. :(

@honnix
Copy link
Contributor Author

honnix commented Jul 26, 2023

The failure doesn't seem to relate to this change.

@eapolinario
Copy link
Collaborator

The failure doesn't seem to relate to this change.

I also believe that they are not related. I kicked off a retry just in case.

Another thing to note is that I was not able to do make doc-requirements.txt on Apple silicon mac because of pip._internal.exceptions.DistributionNotFound: No matching distribution found for tensorflow==2.8.1. I ended up with manually editing doc-requirements.txt. :(

Yeah, the current state of this is not great. I'm thinking about dockering requirements generation, starting with doc-requirements in #1764.

eapolinario
eapolinario approved these changes Jul 26, 2023
View reviewed changes
Copy link
Collaborator

@eapolinario eapolinario left a comment •
edited

Choose a reason for hiding this comment

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

The failures are not related to your change as they are also happening to other PRs.

@eapolinario eapolinario merged commit d51bc0b into master Jul 26, 2023
127 of 130 checks passed
@honnix
Copy link
Contributor Author

honnix commented Aug 1, 2023

Shall we backport this to 1.8.x?

@honnix honnix deleted the no-responses branch August 1, 2023 13:44
honnix added a commit to honnix/flytekit that referenced this pull request Aug 1, 2023
@honnix
Remove dependency on responses (flyteorg#1762)
f3c81f0 
Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
honnix added a commit that referenced this pull request Aug 3, 2023
@honnix @eapolinario
Backport dep fixes to v1.8 (#1774)
ba4f98c 
* Skip problematic pyyaml versions (#1752)

* Skip problematic pyyaml versions

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>

* Regenerate doc-requirements

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>

* Linting

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: eduardo apolinario <eapolinario@users.noreply.github.com>

* Remove dependency on responses (#1762)

Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>

---------

Signed-off-by: eduardo apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com>
Co-authored-by: eduardo apolinario <eapolinario@users.noreply.github.com>
@eapolinario
Copy link
Collaborator

1.9 is being released shortly, so I'd say that it's not necessary. What do you think?

@honnix
Copy link
Contributor Author

honnix commented Aug 4, 2023

@eapolinario Thanks for replying. I have already got help to do a backport and release in #1774 .

fg91 pushed a commit that referenced this pull request Aug 15, 2023
@honnix
Remove dependency on responses (#1762)
eae1f46 
Signed-off-by: Hongxin Liang <honnix@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eapolinario eapolinario eapolinario approved these changes

@wild-endeavor wild-endeavor Awaiting requested review from wild-endeavor wild-endeavor is a code owner

@kumare3 kumare3 Awaiting requested review from kumare3 kumare3 is a code owner

@pingsutw pingsutw Awaiting requested review from pingsutw pingsutw is a code owner

@cosmicBboy cosmicBboy Awaiting requested review from cosmicBboy cosmicBboy is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@honnix @eapolinario