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

Enable usage stats collection for ray.init iff nightly wheels #26461

Merged
merged 8 commits into from
Jul 14, 2022

Conversation

jjyao
Copy link
Collaborator

@jjyao jjyao commented Jul 12, 2022

Why are these changes needed?

For nightly wheels, we want to collect usage stats for local clusters started via ray.init() as well.

Related issue number

Checks

  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@jjyao jjyao marked this pull request as ready for review July 12, 2022 17:29
@ericl ericl removed their assignment Jul 12, 2022
elif usage_stats_enabledness is UsageStatsEnabledness.ENABLED_BY_DEFAULT:

if cli_logger.interactive:
if not cli:
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this path is only hit for nightly wheels. Consider renaming the cli flag and also the message to is_nightly / ...ENABLED_BY_DEFAULT_FOR_NIGHTLY_MESSAGE for clarity.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's more of cli vs ray.init rather than nightly vs non-nightly. Nightly cli and non-nightly cli are the same.

@ericl ericl self-assigned this Jul 13, 2022
@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 13, 2022
@jjyao jjyao removed the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 13, 2022
python/ray/tests/test_usage_stats.py Outdated Show resolved Hide resolved
@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Jul 14, 2022
@jjyao jjyao added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. and removed @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. labels Jul 14, 2022
@jjyao jjyao merged commit 60dd77a into ray-project:master Jul 14, 2022
@jjyao jjyao deleted the jjyao/init branch July 14, 2022 19:14
xwjiang2010 pushed a commit to xwjiang2010/ray that referenced this pull request Jul 19, 2022
…oject#26461)

For nightly wheels, we want to collect usage stats for local clusters started via ray.init() as well.

Signed-off-by: Xiaowei Jiang <[email protected]>
Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
…oject#26461)

For nightly wheels, we want to collect usage stats for local clusters started via ray.init() as well.

Signed-off-by: Stefan van der Kleij <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants