Support creating repositories under organizations during init#656
Support creating repositories under organizations during init#656guysmoilov merged 4 commits intomainfrom
Conversation
When dagshub.init(repo_owner='org', repo_name='repo') is called and the repo doesn't exist, it now compares repo_owner against the current user's username. If they differ, the repo is created under the org via the org API endpoint. Previously it always created repos under the calling user, ignoring the repo_owner parameter entirely. Also forwards the host parameter to create_repo() for correctness. https://claude.ai/code/session_01DaAdy77PhB4KNXRzuQ3FhD
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe pull request enhances repository creation logic to support both user-owned and organization-owned repositories. When a repository is not found, the code now queries the current user and branches the creation path based on whether the repo_owner differs from the current user, with appropriate parameters passed to the repository creation function. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This pull request enhances the repository initialization functionality to support creating repositories under organizations when a repo_owner is specified that differs from the current user. Previously, repositories were always created under the current user's account.
Changes:
- Added logic to fetch current user information and compare with
repo_ownerto determine creation context - Implemented conditional repository creation under organizations vs. current user
- Added
hostparameter tocreate_repo()calls for consistency
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dagshub/common/init.py (1)
110-110:⚠️ Potential issue | 🟡 MinorMissing
hostparameter inget_user_from_tokencall.Line 91 correctly passes
hosttoget_current_user(host=host), but line 110 omits it. For consistency and to ensure the correct host is used (especially with non-default hosts), passhosthere as well.🔧 Proposed fix
- os.environ["MLFLOW_TRACKING_USERNAME"] = UserAPI.get_user_from_token(token).username + os.environ["MLFLOW_TRACKING_USERNAME"] = UserAPI.get_user_from_token(token, host=host).username
🧹 Nitpick comments (3)
dagshub/common/init.py (1)
90-102: Theelsebranch (lines 100-102) appears unreachable.By the time this code executes,
repo_owneris always set:
- If provided explicitly with
repo_name, both are used (line 71-72)- If derived from URL, it's extracted from path parts (line 83)
The condition
if repo_owner:at line 90 will always be true, making lines 100-102 dead code.Additionally, lines 98-99 and 101-102 are duplicates—consider consolidating them.
♻️ Suggested simplification
except RepoNotFoundError: - if repo_owner: - current_user = UserAPI.get_current_user(host=host) - if repo_owner != current_user.username: - log_message( - f'Repository {repo_name} doesn\'t exist, creating it under organization "{repo_owner}".' - ) - create_repo(repo_name, org_name=repo_owner, host=host) - else: - log_message(f"Repository {repo_name} doesn't exist, creating it under current user.") - create_repo(repo_name, host=host) + current_user = UserAPI.get_current_user(host=host) + if repo_owner != current_user.username: + log_message( + f'Repository {repo_name} doesn\'t exist, creating it under organization "{repo_owner}".' + ) + create_repo(repo_name, org_name=repo_owner, host=host) else: log_message(f"Repository {repo_name} doesn't exist, creating it under current user.") create_repo(repo_name, host=host)tests/common/test_init.py (2)
65-70: Test assertions appear incomplete.This test verifies
create_repois called correctly when using a URL, but it's missing assertions that the other tests have:
- No
mock_user_api.get_current_user.assert_called_once()assertion- No
mock_log_message.assert_any_call(...)assertionSince the URL extracts
repo_owner="testuser"which matches the mocked user, this should follow the "current user" path and those assertions would validate the behavior.💡 Suggested additional assertions
def test_init_creates_repo_under_current_user_from_url( mock_repo_api, mock_user_api, mock_create_repo, mock_get_token, mock_log_message ): dagshub.init(url="https://dagshub.com/testuser/my-repo", mlflow=False, dvc=False) + mock_user_api.get_current_user.assert_called_once() mock_create_repo.assert_called_once_with("my-repo", host="https://dagshub.com") + mock_log_message.assert_any_call( + "Repository my-repo doesn't exist, creating it under current user." + )
41-70: Consider adding a test for org creation via URL.The tests cover owner/name args and URL with matching user, but there's no test for creating a repo under an org when using a URL (e.g.,
url="https://dagshub.com/my-org/my-repo"wheremy-orgdiffers from current user). This would ensure the org creation path works correctly with URL input.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignoredagshub/common/init.pytests/common/test_init.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (3.11)
- GitHub Check: build (3.13)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
- GitHub Check: build (3.10)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/common/test_init.py (3)
dagshub/common/api/repo.py (2)
RepoNotFoundError(43-44)get_repo_info(96-109)dagshub/common/api/user.py (3)
username(68-69)get_current_user(64-65)get_user_from_token(37-61)dagshub/common/init.py (1)
init(23-151)
dagshub/common/init.py (2)
dagshub/common/api/user.py (3)
UserAPI(21-88)get_current_user(64-65)username(68-69)dagshub/common/helpers.py (1)
log_message(100-107)
🪛 Ruff (0.15.1)
tests/common/test_init.py
[warning] 42-42: Unused function argument: mock_repo_api
(ARG001)
[warning] 42-42: Unused function argument: mock_get_token
(ARG001)
[warning] 54-54: Unused function argument: mock_repo_api
(ARG001)
[warning] 54-54: Unused function argument: mock_get_token
(ARG001)
[warning] 66-66: Unused function argument: mock_repo_api
(ARG001)
[warning] 66-66: Unused function argument: mock_user_api
(ARG001)
[warning] 66-66: Unused function argument: mock_get_token
(ARG001)
[warning] 66-66: Unused function argument: mock_log_message
(ARG001)
🔇 Additional comments (4)
.gitignore (1)
129-129: LGTM!Adding
*.imlto the IntelliJ section is appropriate—these are IDE-specific module files that should not be tracked.tests/common/test_init.py (3)
9-38: LGTM on fixtures.The fixtures correctly mock the dependencies needed for testing the
initfunction. The unused argument warnings from static analysis are false positives—these fixtures are required to activate the patches even if not directly referenced in test bodies.
41-50: LGTM!This test correctly verifies that when
repo_ownerdiffers from the current user, the repository is created under the organization with the appropriate parameters and log message.
53-62: LGTM!This test correctly verifies that when
repo_ownermatches the current user, the repository is created under the user (withoutorg_name) and logs the appropriate message.
kbolashev
left a comment
There was a problem hiding this comment.
LGTM!
Actual bug report, from messages:
dagshub.init(owner_name='bla', repo_name='foo') always creates the repo as owned by the calling user, doesn't attempt to create it under the bla org or verify that it actually is an org and that the user can create repos under it, etc.
Summary
Enhanced the repository initialization logic to support creating repositories under organizations when a
repo_owneris specified, rather than always creating under the current user.Key Changes
repo_owneris specified and differs from the current user, create the repository under the specified organizationhostparameter tocreate_repo()calls to ensure consistency with the API hostImplementation Details
UserAPI.get_current_user()to retrieve the current user's information for comparisonorg_name=repo_ownerparameter tocreate_repo()when creating under an organizationrepo_owneris specified or when it matches the current userhttps://claude.ai/code/session_01DaAdy77PhB4KNXRzuQ3FhD
Implementation Details
Updated dagshub/common/init.py to make repository creation organization-aware when a repository is missing:
Tests added (tests/common/test_init.py):
Supporting changes (unchanged behavior elsewhere):
Files modified: