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

(enhancement): Move RayLogger out of non-distributed modules #1686

Merged
merged 8 commits into from
Oct 17, 2022
Merged

(enhancement): Move RayLogger out of non-distributed modules #1686

merged 8 commits into from
Oct 17, 2022

Conversation

jaidisido
Copy link
Contributor

@jaidisido jaidisido commented Oct 14, 2022 •
edited

Feature or Bugfix

  • Enhancement

Currently the RayLogger is instantiated in several places in the non-distributed modules. This change moves this initialisation into the ray_remote method instead, reducing LOC and separating concerns. Moreover, the PR adds the ability to turn the logger on or off based on the config.log_to_driver flag.

Detail

  • Move RayLogger out of non-distributed modules and into the ray_remote decorator
  • Enable turning the logger on/off

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jaidisido jaidisido self-assigned this Oct 14, 2022
@jaidisido jaidisido added enhancement New feature or request major release Will be addressed in the next major release labels Oct 14, 2022
@jaidisido jaidisido added this to the 3.0.0 milestone Oct 14, 2022
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 6af698f
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

jaidisido
jaidisido commented Oct 14, 2022
View reviewed changes
awswrangler/distributed/ray/_core.py Outdated Show resolved Hide resolved
awswrangler/_config.py Show resolved Hide resolved
awswrangler/_config.py Outdated Show resolved Hide resolved
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 6af698f
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido jaidisido added this to In progress in AWS SDK for pandas roadmap Oct 14, 2022
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 6af698f
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 0a53434
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 86c5723
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 944866f
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 86c5723
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 86c5723
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 7315bfd
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 7315bfd
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 7315bfd
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido jaidisido moved this from In progress to In Review in AWS SDK for pandas roadmap Oct 14, 2022
malachi-constant
malachi-constant approved these changes Oct 14, 2022
View reviewed changes
Copy link
Contributor

@malachi-constant malachi-constant left a comment

Choose a reason for hiding this comment

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

Thanks @jaidisido

awswrangler/_config.py Outdated Show resolved Hide resolved
awswrangler/distributed/ray/_core.py


def ray_get(futures: List[Any]) -> List[Any]:
def ray_logger(function: Callable[..., Any]) -> Callable[..., Any]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, was wondering if this could be put in a decorator, thanks.

awswrangler/distributed/ray/_core.py Outdated Show resolved Hide resolved
awswrangler/distributed/ray/_register.py Show resolved Hide resolved
jaidisido
jaidisido commented Oct 14, 2022
View reviewed changes
awswrangler/distributed/ray/_core.py
@@ -24,27 +24,33 @@ def __init__(
):
Copy link
Contributor Author

@jaidisido jaidisido Oct 14, 2022 •
edited

Choose a reason for hiding this comment

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

Also the log_level is set to INFO by default with no way for the user to change it when most of our loggers are set to debug, not sure what the default should be

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say INFO by default, but we should definitely allow to override it

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 6152345
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 6152345
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: ad0c942
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubDistributedCodeBuild6-jWcl5DLmvupS
  • Commit ID: 9d17457
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido jaidisido merged commit c0e75b9 into release-3.0.0 Oct 17, 2022
@jaidisido jaidisido deleted the enhancement/move-ray-logger branch October 17, 2022 14:37
@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubLoadTests5656BB24-s6u9F3qN9oFy
  • Commit ID: 9d17457
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@malachi-constant
Copy link
Contributor

AWS CodeBuild CI Report

  • CodeBuild project: GitHubStandardCodeBuild8C06-llutOAimTATs
  • Commit ID: 9d17457
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@jaidisido jaidisido moved this from In Review to Done in AWS SDK for pandas roadmap Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kukushking kukushking kukushking approved these changes

@malachi-constant malachi-constant malachi-constant approved these changes

@LeonLuttenberger LeonLuttenberger LeonLuttenberger approved these changes

@cnfait cnfait Awaiting requested review from cnfait

Assignees

@jaidisido jaidisido

Labels
enhancement New feature or request major release Will be addressed in the next major release
Projects
AWS SDK for pandas roadmap
Done
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jaidisido @malachi-constant @kukushking @LeonLuttenberger