Merged
Conversation
… while to respond
…aseClient` method This is now possible through the implementation of the `expected_code` keyword argument and the fact that the Hub's response now contains the log resource.
📝 WalkthroughWalkthroughThis PR introduces a new ClientAuth authentication mechanism, adds Client resource management with full CRUD operations, refactors status fields using unified ProcessStatus typing, restructures bucket and file models with enhanced metadata tracking, updates Node and Analysis models with additional fields, and deprecates RobotAuth in favor of ClientAuth. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flame_hub/_storage_client.py (1)
60-63:⚠️ Potential issue | 🟠 Major
apply_upload_file_defaultsoverwrites user-specifiedcontent_typevalues.
UploadFileis aTypedDict(runtime dict), andhasattr(uf, "content_type")checks for object attributes, not dict keys. This returnsFalseeven when the key exists in the dict, causing user-provided values to be replaced. Use"content_type" not in ufinstead to check for key presence.Suggested fix
- if not hasattr(uf, "content_type") or uf["content_type"] is None: + if "content_type" not in uf or uf["content_type"] is None: uf["content_type"] = "application/octet-stream"
🤖 Fix all issues with AI agents
In @.env.test:
- Line 10: Dotenv-linter complains about key ordering for PYTEST_HUB_VERSION;
fix it by moving the PYTEST_HUB_VERSION entry so it appears above the
PYTEST_STORAGE_BASE_URL entry in the env file, preserving the exact key/value
and ensuring the rest of the key order remains unchanged.
In `@tests/conftest.py`:
- Around line 302-308: The nginx fixture in function nginx currently fetches
nginx.conf from a moving branch ref ("refs/heads/master"); change the httpx.get
call to use a pinned ref by reading an environment variable (e.g.,
NGINX_CONF_REF) with a sensible default set to a stable tag or commit hash, and
update the URL construction in the nginx function so it interpolates that ref
instead of hardcoding "refs/heads/master"; ensure the env var is documented or
set in tests so CI uses the stable ref while allowing overrides for local
testing.
In `@tests/helpers.py`:
- Around line 38-42: The function next_random_number calls random.randint when
is_integer=True but accepts float bounds, causing TypeError; modify
next_random_number to coerce/validate bounds before calling random.randint by
converting lower to math.ceil(lower) and upper to math.floor(upper) (import
math) and then ensure lower_int <= upper_int else raise ValueError; keep the
rest of the logic intact and reference next_random_number and random.randint
when making the change.
🧹 Nitpick comments (2)
flame_hub/__init__.py (1)
27-28: Scope DeprecationWarning filter to this library only.Line 27-28 changes the global warnings behavior for all modules, which can be noisy for host applications. Consider scoping the filter to
flame_hubinstead.🔧 Suggested adjustment
-warnings.simplefilter("default", DeprecationWarning) +warnings.filterwarnings( + "default", + category=DeprecationWarning, + module=r"^flame_hub(\.|$)", +)flame_hub/_auth_flows.py (1)
66-69: Add stacklevel so the deprecation warning points to caller.Line 66-69 will currently point to internal code. Adding
stacklevel=2makes the warning actionable for users.🔧 Suggested tweak
warnings.warn( "'RobotAuth' is deprecated and will be removed in a future version. Please use 'ClientAuth' instead.", category=DeprecationWarning, + stacklevel=2, )
| PYTEST_ASYNC_MAX_RETRIES=5 | ||
| PYTEST_ASYNC_RETRY_DELAY_MILLIS=500 | ||
| PYTEST_HUB_VERSION=0.8.21 | ||
| PYTEST_HUB_VERSION=0.8.25 |
There was a problem hiding this comment.
Fix dotenv-linter key order warning.
Line 10 is flagged by dotenv-linter for key ordering. Move PYTEST_HUB_VERSION above PYTEST_STORAGE_BASE_URL.
🔧 Suggested reorder
PYTEST_CORE_BASE_URL=http://localhost:3000/core/
PYTEST_AUTH_BASE_URL=http://localhost:3000/auth/
-PYTEST_STORAGE_BASE_URL=http://localhost:3000/storage/
+PYTEST_HUB_VERSION=0.8.25
+PYTEST_STORAGE_BASE_URL=http://localhost:3000/storage/
PYTEST_ADMIN_USERNAME=admin
PYTEST_ADMIN_PASSWORD=start123
PYTEST_DEFAULT_MASTER_IMAGE=python/base
PYTEST_ASYNC_MAX_RETRIES=5
PYTEST_ASYNC_RETRY_DELAY_MILLIS=500
-PYTEST_HUB_VERSION=0.8.25📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| PYTEST_HUB_VERSION=0.8.25 | |
| PYTEST_CORE_BASE_URL=http://localhost:3000/core/ | |
| PYTEST_AUTH_BASE_URL=http://localhost:3000/auth/ | |
| PYTEST_HUB_VERSION=0.8.25 | |
| PYTEST_STORAGE_BASE_URL=http://localhost:3000/storage/ | |
| PYTEST_ADMIN_USERNAME=admin | |
| PYTEST_ADMIN_PASSWORD=start123 | |
| PYTEST_DEFAULT_MASTER_IMAGE=python/base | |
| PYTEST_ASYNC_MAX_RETRIES=5 | |
| PYTEST_ASYNC_RETRY_DELAY_MILLIS=500 |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 10-10: [UnorderedKey] The PYTEST_HUB_VERSION key should go before the PYTEST_STORAGE_BASE_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
In @.env.test at line 10, Dotenv-linter complains about key ordering for
PYTEST_HUB_VERSION; fix it by moving the PYTEST_HUB_VERSION entry so it appears
above the PYTEST_STORAGE_BASE_URL entry in the env file, preserving the exact
key/value and ensuring the rest of the key order remains unchanged.
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Deprecations
Documentation