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 explicit asgiref dependency #1667

Merged
merged 1 commit into from
Mar 6, 2023
Merged

Conversation

RobbeSneyders
Copy link
Member

@RobbeSneyders RobbeSneyders commented Mar 5, 2023

We use asgiref as a direct dependency

from asgiref.sync import async_to_sync

So should add it explicitly as well.

This issue is hidden in our tests since we install all extras. To prevent this, we need to address #1389.

@RobbeSneyders RobbeSneyders added this to the Connexion 3.0 milestone Mar 5, 2023
@RobbeSneyders RobbeSneyders requested a review from Ruwann March 5, 2023 19:44
@RobbeSneyders RobbeSneyders force-pushed the bugfix/asgiref-dependency branch from 9afc248 to e95b5d6 Compare March 5, 2023 19:48
@coveralls
Copy link

Pull Request Test Coverage Report for Build 4337711503

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.238%

Totals Coverage Status
Change from base Build 4321937364: 0.0%
Covered Lines: 3268
Relevant Lines: 3505

💛 - Coveralls

@coveralls
Copy link

coveralls commented Mar 5, 2023

Pull Request Test Coverage Report for Build 4347858504

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.162%

Totals Coverage Status
Change from base Build 4347813368: 0.0%
Covered Lines: 3270
Relevant Lines: 3510

💛 - Coveralls

Copy link
Member

@Ruwann Ruwann left a comment

Choose a reason for hiding this comment

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

Is this the lowest version possible? Is it worth it to lower this if possible?

@RobbeSneyders RobbeSneyders force-pushed the bugfix/asgiref-dependency branch from e95b5d6 to 4113848 Compare March 6, 2023 20:50
@RobbeSneyders
Copy link
Member Author

I lowered it to match the lower bound that Flask defines.

@RobbeSneyders RobbeSneyders force-pushed the bugfix/asgiref-dependency branch from 4113848 to b300c56 Compare March 6, 2023 20:53
@RobbeSneyders
Copy link
Member Author

That didn't work. Bumped it back up to match the lower bound defined by uvicorn.

@RobbeSneyders RobbeSneyders merged commit cd64611 into main Mar 6, 2023
@RobbeSneyders RobbeSneyders deleted the bugfix/asgiref-dependency branch March 6, 2023 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants