Skip to content

Conversation

@dsblank
Copy link
Collaborator

@dsblank dsblank commented Dec 3, 2025

Summary of Changes

1. Fixed comet_ml Package Compatibility

  • Updated imports to work with the current comet_ml package:
    • format_urlmerge_url from comet_ml.utils
    • get_comet_api_client → import from comet_ml directly
    • url_join → moved from comet_ml.connection to comet_ml.utils
  • Updated format_url() usage to merge_url() with dict parameters

2. Added MLflow Authentication Error Handling

  • Added RestException import and handling
  • Detects 401 authentication errors when connecting to MLflow stores
  • Shows helpful error messages with instructions for Databricks and other authenticated stores

3. Updated Environment Variable

  • Changed from MLFLOW_TRACKING_TOKEN to DATABRICKS_TOKEN throughout:
    • Error messages
    • README documentation
    • All references

4. Updated URLs and Branding

  • Changed all comet.ml URLs to comet.com:
    • Documentation links
    • Support links
    • Example URLs in README
  • Updated user-facing text to reference "Comet ML" or "Comet.com" as appropriate
  • Fixed config file path reference from ~/.comet.ini to ~/.comet.config

5. Fixed Databricks Token Generation Instructions

  • Removed incorrect reference to #setting/account
  • Added step-by-step instructions:
    1. Click user profile icon
    2. Select "User Settings"
    3. Go to "Access Tokens" tab
    4. Click "Generate New Token"

The codebase is now compatible with the current comet_ml package and provides clearer guidance for Databricks authentication.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the codebase to work with modern versions of comet_ml and mlflow packages, while improving authentication error handling and updating branding from comet.ml to comet.com.

Key Changes:

  • Updated comet_ml package imports to use current API structure (e.g., format_urlmerge_url, relocated utility imports)
  • Added comprehensive MLflow authentication error handling with helpful guidance for Databricks and other authenticated stores
  • Changed environment variable from MLFLOW_TRACKING_TOKEN to DATABRICKS_TOKEN and updated all documentation

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
comet_for_mlflow/comet_for_mlflow.py Updated imports for modern comet_ml API, added authentication error handling with try-catch blocks, changed function calls from format_url() to merge_url(), updated all user-facing text from comet.ml to comet.com
comet_for_mlflow/utils.py Updated documentation URL from comet.ml to comet.com
tests/test_comet_for_mlflow.py Updated import path for url_join and changed server address URL from comet.ml to comet.com
examples/keras-example/run.py Added Databricks-specific configuration for tracking URI and experiment path
README.md Updated all example URLs from comet.ml to comet.com, added comprehensive authentication section with Databricks token generation instructions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@Lothiraldan Lothiraldan left a comment

Choose a reason for hiding this comment

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

LGTM

@dsblank dsblank merged commit 132e180 into master Dec 4, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants