move icon indicating copy to clipboard operation
move copied to clipboard

[Movey] Call Movey API to increase download count

Open ea-open-source opened this issue 3 years ago • 8 comments
trafficstars

Motivation

According to Sam, we moved the call to Movey into this pull request.

How to use it

As usual any build action that involves cloning from github will also calls to Movey to increase download count metre.

Note: you could use the flag--skip-movey to skip this call, although it has negligible impact on build performance. Example: move build --skip-movey

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

cargo test --package move-package --lib increase_movey_download_count

ea-open-source avatar Jul 29 '22 02:07 ea-open-source

I have to say I am still confused what this PR does... I though this addition is to avoid increasing GitHub's download count when uploading the repo content to Movey (so that the counter value is not artificially inflated by multiple uploads). Looking at the code, though, it seems like we are actually incrementing the counter (which would otherwise not be incremented as part of the upload?), so I am not even sure if I got the notion of the counter right... Could you please provide some additional explanation?

For this PR it does increase the counter but it's the download counter whenever a move package in move.toml is resolved by resolution graph. A new thread will be spun to send a request to Movey whenever download_and_upload_if_remote is called so by default it will skip already downloaded packages.

ea-open-source avatar Aug 17 '22 03:08 ea-open-source

Adding @tnowacki as a reviewer here as this PR involves a change to package resolver and I am not sure if spawning a thread and calling to Movey on (seemingly) each dependency check (for already downloaded dependencies) during resolution graph building is the right strategy here.

awelc avatar Aug 17 '22 17:08 awelc

Adding @tnowacki as a reviewer here as this PR involves a change to package resolver and I am not sure if spawning a thread and calling to Movey on (seemingly) each dependency check (for already downloaded dependencies) during resolution graph building is the right strategy here.

I think you misunderstood the logic, calling to Movey only happens when download_and_update_if_remote is called and this only happens after the dependency tree is resolved. So in essence, Movey only increases download counts when users download a package from remote.

ea-open-source avatar Aug 18 '22 08:08 ea-open-source

@awelc @tnowacki would you guys help us review this PR?

ea-open-source avatar Aug 31 '22 07:08 ea-open-source

In the PR description, you have

Example: move movey-upload --skip-movey

I think this should be move build --skip-movey? I'm not sure how this control flow path will ever be hit in movey-upload?

tnowacki avatar Sep 09 '22 17:09 tnowacki

@awelc @tnowacki There are multiple reasons why we made this explicitly opt-in:

  1. Most of our existing packages were crawled from Github and they include a lot of unused or for practice packages. We collect these information to generate Movey info page about truly important packages, to set them apart from automatically crawled packages.
  2. In order to do this, we aim to integrate ourselves into the Move ecosystem as the official Move package repository. Therefore we set opt-in as the default behavior.

ea-open-source avatar Sep 12 '22 09:09 ea-open-source

Additionally, the --skip-movey flag is presented in the description of this PR as an addition to the move movey-upload, but it's part of build config and (I think) move movey-upload does not even do any building. Please clarify.

In the PR description, you have

Example: move movey-upload --skip-movey

I think this should be move build --skip-movey? I'm not sure how this control flow path will ever be hit in movey-upload?

Yes, It should be move build --skip-movey not move movey-upload --skip-movey. Our bad.

ea-open-source avatar Sep 12 '22 09:09 ea-open-source

Movey is a centralized registry for Move packages but not an independent repository ("its own custom source provider"). Among the reasons for not being a repository are (1) developers likely prefer Github and this may hamper adoption, and (2) there would be a lot more engineering involved and this would take time and be redundant ("reinventing the wheel"), (3) this would take focus away from features that add value not found elsewhere.

Perhaps what is being suggested here is that Movey should provide a pointer to existing on-chain locations for packages. And indeed, we do have that on our roadmap. At first these links might be informational (no guarantees) later they can be validated and certified by us.

@tnowacki perhaps we are missing something. What is the compelling advantage?

lwsinclair avatar Sep 14 '22 02:09 lwsinclair

After further discussions about package dependencies resolving, we have released a new tool MoveyCLI. This PR is no longer needed. Closing.

ea-open-source avatar Feb 17 '23 07:02 ea-open-source