Skip to content

Conversation

@scbedd
Copy link
Member

@scbedd scbedd commented Mar 31, 2021

@seankane-msft

This is related to this issue.

Can you poke holes in my logic? We don't really have anything to key off to identify one of these "optional" requirements that may or may not be present. So it's not really a case of extending create_package_and_install.py to identify additional requirements.

To solve this generically without messing with anyone's day-to-day development, I created a new tox environment whl_no_aio. This is essentially a retread of whl (aka the default) tox environment. The only difference is that have an error-ignored line (notice the leading -) that attempts uninstall of aiohttp prior to invoking any of the install or test commands.

{envbindir}/python -m pip freeze
pytest \
{[testenv]default_pytest_params} \
--ignore-glob='*_async.py' \
Copy link
Member Author

Choose a reason for hiding this comment

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

Not having this means that async tests implode with

image

@seankane-msft
Copy link
Contributor

Within the create_package_and_install.py file, can we add a flag for this toxenv where we add a clause like the following to verify that the uninstallation was successful:

try:
    import aiohttp
    raise Exception("aiohttp was not successfully installed") 
except ImportError:
    pass

@scbedd
Copy link
Member Author

scbedd commented Apr 1, 2021

I see what you're going after here. I'll add something like it.

@scbedd
Copy link
Member Author

scbedd commented Apr 1, 2021

@seankane-msft added!

Copy link
Contributor

@seankane-msft seankane-msft left a comment

Choose a reason for hiding this comment

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

:shipit:

clean imports

Co-authored-by: Sean Kane <68240067+seankane-msft@users.noreply.github.com>
@scbedd scbedd enabled auto-merge (squash) April 1, 2021 17:14
@scbedd
Copy link
Member Author

scbedd commented Apr 5, 2021

/azp run python - core - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scbedd scbedd disabled auto-merge April 5, 2021 21:57
@scbedd scbedd enabled auto-merge (squash) April 5, 2021 21:58
@scbedd scbedd merged commit 4485013 into Azure:master Apr 5, 2021
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.

2 participants