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

[poc] Cache in local Artifactory sources downloaded by Conan #8211

Closed
wants to merge 12 commits into from

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Dec 15, 2020

Changelog: (Feature | Fix | Bugfix): Describe here your pull request
Docs: https://github.com/conan-io/docs/pull/XXXX

Changes in this PR:

  • CachedFileDownloader writes a file .cached_files with the mapping between hashed files and original URLs (it will be used by the JFrog CLI to upload selected files to Artifactory and property to identify them).
  • Introduces new ArtifactoryCacheDownloader class that will be added to the downloaders when the config variable storage.sources_backup is set. This class is not added for the RestClientsV1/V2, so files coming from Conan remotes won't be retrieved from this backup Artifactory.

... it is adding tests using pytest style, and they are run by the CI.

closes #6876
closes conan-io/conan-center-index#2715

@jgsogo jgsogo force-pushed the feat/artifactory_cache branch from 6254331 to 10dfd15 Compare December 15, 2020 16:45
@jgsogo jgsogo force-pushed the feat/artifactory_cache branch from 7852afa to cd8e146 Compare December 15, 2020 16:57
@jgsogo jgsogo added this to the 1.33 milestone Dec 15, 2020
@jgsogo jgsogo requested review from czoido and memsharded December 15, 2020 18:01
@jgsogo jgsogo changed the title [poc][wip] Cache in local Artifactory sources downloaded by Conan [poc] Cache in local Artifactory sources downloaded by Conan Dec 15, 2020
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

It is looking very good

conans/client/downloaders/artifactory_cache_downloader.py Outdated Show resolved Hide resolved
conans/client/downloaders/artifactory_cache_downloader.py Outdated Show resolved Hide resolved
conans/client/downloaders/cached_file_downloader.py Outdated Show resolved Hide resolved
conans/client/downloaders/cached_file_downloader.py Outdated Show resolved Hide resolved
sha1=sha1, sha256=sha256, **kwargs)
with self._lock(self._cache_mapping):
mapping_filename = os.path.join(self._cache_folder, self._cache_mapping)
with open(mapping_filename, "a") as f:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be better to explicitly store the "user-download" in this data file? Are you going to filter later based on URLs only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the filter uses the URLs from the remotes (implemented here: https://github.com/conan-io/conan-jfrog-cli-plugin/blob/87eacc796918784e013a5aefbf31a2885228cc25/commands/backup_sources.go#L140).

Anyway, there was a fortunate coincidence, as usage of the downloaders from our rest clients (calls to Conan remotes) and from tools.get/download is totally isolated, this ArtifactoryCache middleware is only added for the tools.get/download calls, all of them should probably user_downloads.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

I think it is good. Maybe it need to wait until the plugin is ready, lets see how/when to release this.

downloader = FileDownloader(requester=requester, output=output, verify=verify, config=config)

if use_rt_cache and config.sources_backup:
Copy link
Member

Choose a reason for hiding this comment

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

This was confusing while I was reading it: you can call run_downloader(..., use_rt_cache=True,....) but the sources backup cache might not be used because config.backup_sources is not configured. Maybe this could be considered:

  • Rename use_rt_cache => backup_sources_enabled or something like that.
  • Just use use_cache and config.sources_backup

Just a very minor detail, if the UI could be improved, great, otherwise, not a big issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was using the same approach as the download cache with use_cache argument and then config.download_cache.

use_cache is also used in the context of the REST APIs, where use_rt_cache must be always False, so we need both of them.

Renaming, ok, but then let's rename both?

  • use_rt_cache => sources_backup_enabled
  • use_cache => download_cache_enabled

@jgsogo jgsogo self-assigned this Dec 22, 2020
@jgsogo
Copy link
Contributor Author

jgsogo commented Dec 22, 2020

After feedback and demo, I will resume the implementation (here and JFrog CLI plugin).

@samuel-emrys
Copy link
Contributor

@jgsogo are you able to provide any updates on how this is going? We're looking to mirror CCI in an offline environment and this looks like it could be useful to us.

@prince-chrismc
Copy link
Contributor

We had a similar requirement. Build subnet had no internet access.

We did a little #6287 and host files pointing to artifactory to get ourselves started

This was referenced Jan 19, 2022
@barower
Copy link

barower commented Jan 28, 2022

What's the status? In our company we use closed environment with proxy to conan.io and it works, but only as long as there are binary packages available. A method to mirror sources without internally forking conan-center-index would be much appreciated

@memsharded
Copy link
Member

@barower the idea is still valid and we will probably resume working on it after 2.0, which is our current focus (we are in alpha-3, will still need to go through beta and GA)

@chenyuanrun
Copy link

We need this feature too, our developing enviroment can not connect to internet directly and the proxy can only access to a limit of web site, it will be nice if conan has a single site to download all sources from and we can proxy to that site to get the sources.

@chenyuanrun
Copy link

We need this feature too, our developing enviroment can not connect to internet directly and the proxy can only access to a limit of web site, it will be nice if conan has a single site to download all sources from and we can proxy to that site to get the sources.

Cargo (the package manager for rust) can download all sources from a single location (config by dl field of config.json in package index):
https://doc.rust-lang.org/cargo/reference/registries.html#index-format

@memsharded
Copy link
Member

@chenyuanrun I think this is not the same case, the problem is that Conan package sources are external, in Github, sourceforge, etc, while cargo crates store the sources in the crate itself.

@maximiliank
Copy link

Maybe, a solution would be to setup a http proxy on the machine to redirect the requests to the external sources to your mirror? It was suggested at https://stackoverflow.com/a/35371232/9759769 and I tried to set it up, but the rewrite rules are not working, see my example here https://stackoverflow.com/questions/73795692/apache2-proxy-and-rewrite-setup. In case you have an idea how to get the rewrite rules working, please let me know.

@memsharded
Copy link
Member

Yes, a network solution like defining a proxy would be doable. Still, it seems reasonable to implement something core in Conan, as we know that this kind of setup can be complicated to get buy-in, complicated to maintain over-time, etc.

@samuel-emrys
Copy link
Contributor

Now that conan 2.0 has been released, it would be good to get a timeline on when this might be considered. I'm using a proxy at the moment, but it has deficiencies so I'm super keen to see this capability merged in.

@memsharded memsharded closed this May 5, 2023
@samuel-emrys
Copy link
Contributor

@memsharded is this no longer planned, or is it being progressed elsewhere?

@prince-chrismc
Copy link
Contributor

I do not know when it was merged but @RubenRBS has been working on this with his recent commits
https://github.com/conan-io/conan/commits?author=RubenRBS

@memsharded
Copy link
Member

This has already been implemented, in a different PR #13461, and it is already quietly released since 2.0.3, but undocumented. We are currently testing and improving it for ConanCenter, we will launch it soon after this testing.

@samuel-emrys
Copy link
Contributor

That's great news, I'm excited to test it out. Thanks for all the work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants