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

Add warning when downloading recipes from bintray #9416

Merged
merged 5 commits into from Aug 23, 2021
Merged

Add warning when downloading recipes from bintray #9416

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f917833
Add warning when downloading recipes from bintray
danimtb Aug 12, 2021
6c295cc
fix msg format
danimtb Aug 12, 2021
091f578
change test
danimtb Aug 12, 2021
b85a5f8
Use mocked remote
danimtb Aug 18, 2021
a780ffa
Update conans/client/graph/proxy.py
danimtb Aug 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions conans/client/graph/proxy.py
View file
Open in desktop
Expand Up @@ -113,6 +113,11 @@ def _download_recipe(self, layout, ref, output, remotes, remote, recorder):
def _retrieve_from_remote(the_remote):
output.info("Trying with '%s'..." % the_remote.name)
# If incomplete, resolve the latest in server
if "https://conan.bintray.com" in the_remote.url:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should declare that URL as a global variable in this module like:

BINTRAY_DEPRECATED_URL = "https://conan.bintray.com"

# .....

if the_remote.url.startswith(BINTRAY_DEPRECATED_URL):
      # whatever

Then, it should be easier to use it in any test.

Copy link
Contributor

@jgsogo jgsogo Aug 13, 2021 •
edited

Choose a reason for hiding this comment

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

When using Bintray, if you clicked the set me up button it didn't show you the https://conan.bintray.com/... URL, but https://bintray.com/conan.... (I cannot remember exactly the path)... and many people had that one configured. Do you remember? Do we want to check also for that URL?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I'd say to check fur bintray.com, that should be good enough for both cases, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

the URL https://bintray.com/conan.... will be useless as it is not working (only conan.bintray.com is still operational), so checking for it doesn't make sense as it won't work from the beginning and would never get this warning message

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a very good reason indeed 😄

output.warn("Remote https://conan.bintray.com is deprecated and will be shut down "
"soon.")
output.warn("Please use the new 'conancenter' default remote "
"(https://center.conan.io).")
Copy link
Contributor

Choose a reason for hiding this comment

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

For these messages, I really find it useful to provide a copy/paste solution to users:

Suggested change
output.warn("Please use the new 'conancenter' default remote "
"(https://center.conan.io).")
output.warn("Please use the new 'conancenter' default remote. Add it to your remotes with "
"'conan remote add --insert 0 conancenter https://center.conan.io'.")

_ref = self._remote_manager.get_recipe(ref, the_remote)
output.info("Downloaded recipe revision %s" % _ref.revision)
recorder.recipe_downloaded(ref, the_remote.url)
Expand Down
15 changes: 15 additions & 0 deletions conans/test/integration/command/install/install_test.py
View file
Open in desktop
Expand Up @@ -447,3 +447,18 @@ def test_create_cli_override(self, client):
"test_package/conanfile.py": GenConanfile().with_test("pass")})
client.run("create . pkg/0.1@ --require-override=zlib/2.0")
assert "zlib/2.0: Already installed" in client.out


def test_install_bintray_warning():
"""
IMPORTANT: This test is actually using https://conan.bintray.com
Warning is only displayed when downloading a recipe from the remote
"""
client = TestClient()
client.run("remote add conan-center https://conan.bintray.com")
Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to avoid using the real bintray remote with a test server using the same URL. However, this lead to modifications to the TestServer class and others in the requester...

Anyway, I think it could be useful to have a test that tracks the availability of the remote so we can drop the message once it is no longer available 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

Then, in my opinion, I think we should use a "fake" Bintray URL and check the output:

    client = TestClient()
    fake_bintray_url = "https://myfakebintray.com"
    with patch.object(proxy, "BINTRAY_DEPRECATED_URL", return_value=fake_bintray_url):
         client.run("remote add conan-center %s" % fake_bintray_url)
         # .....

What do you think @danimtb?

EDIT: perhaps that client.run() can be outside of the patch

client.run("install zlib/1.2.8@conan/stable -r conan-center")
assert "WARN: Remote https://conan.bintray.com is deprecated and will be shut down " \
"soon" in client.out
client.run("install zlib/1.2.8@conan/stable -r conan-center -s build_type=Debug")
assert "WARN: Remote https://conan.bintray.com is deprecated and will be shut down " \
"soon" not in client.out