Release v0.6.0: result handling, messaging resilience, and status sync#22
Merged
Conversation
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
# Conflicts: # flamesdk/flame_core.py # flamesdk/resources/client_apis/clients/data_api_client.py # flamesdk/resources/client_apis/clients/po_client.py # flamesdk/resources/client_apis/clients/storage_client.py # flamesdk/resources/client_apis/storage_api.py
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
# Conflicts: # flamesdk/flame_core.py # flamesdk/resources/rest_api.py # flamesdk/resources/utils/logging.py # pyproject.toml
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
…sending messages Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: Nightknight3000 <alexander.roehl@uni-tuebingen.de>
Co-authored-by: antidodo <albin2993@gmail.com>
Co-authored-by: antidodo <albin2993@gmail.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR updates the SDK's core parameter signatures, implements log-level filtering infrastructure, refactors the intermediate-data query API, adds filename support to result submission, standardizes HTTP error handling across all clients, and improves message broker resilience with retries and deduplication. ChangesSDK Core Refactoring and Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
flamesdk/resources/client_apis/clients/data_api_client.py (1)
63-68:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove the S3 request into the
tryblock.Lines 63-65 execute
asyncio.run(self.client.get(...))outside thetryblock. When this request raisesConnectErrororTimeoutException, the exception handler at lines 68-70 cannot catch it. This inconsistency leaves transient S3 failures unhandled, unlike the FHIR path (lines 47-52) and the helper method_get_s3_dataset_names(lines 76-78), which both wrap the request in thetryblock.Proposed fix
if (len(s3_keys) == 0) or (res_name in s3_keys): - response = asyncio.run(self.client.get(f"{source['name']}/s3/{res_name}", - headers=[('Connection', 'close')], - timeout=Timeout(5, write=None, read=None))) try: + response = asyncio.run(self.client.get(f"{source['name']}/s3/{res_name}", + headers=[('Connection', 'close')], + timeout=Timeout(5, write=None, read=None))) response.raise_for_status() except (HTTPStatusError, ConnectError, TimeoutException) as e: self.flame_logger.raise_error(f"Failed to retrieve s3 data for key {res_name} " f"from source {source['name']}: {repr(e)}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@flamesdk/resources/client_apis/clients/data_api_client.py` around lines 63 - 68, The S3 GET call is made outside the exception handler so ConnectError/TimeoutException escape the except; move the asyncio.run(self.client.get(...)) invocation into the same try block that calls response.raise_for_status() (the block handling HTTPStatusError, ConnectError, TimeoutException) so transient S3 failures are caught consistently; mirror the pattern used for the FHIR path and _get_s3_dataset_names by wrapping the request and subsequent response.raise_for_status() together inside the try surrounding those exception types (update the block around the S3 request in data_api_client.py where response is assigned).flamesdk/resources/client_apis/clients/storage_client.py (1)
159-198:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSwitch the required-parameter guards to
query.This method was refactored to accept
query, but Lines 175-178 still testid, which is the Python built-in here and therefore neverNone. Calls likeget_intermediate_data(type="global")now fall through to/_/Noneinstead of failing fast.Suggested fix
- if (type == "global") and (id is None): + if (type == "global") and (query is None): self.flame_logger.raise_error("Global intermediate data retrieval requires storage id specification") - if (type == "local") and (id is None) and (tag is None): + if (type == "local") and (query is None) and (tag is None): self.flame_logger.raise_error("For local data a tag or storage id has to be specified")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@flamesdk/resources/client_apis/clients/storage_client.py` around lines 159 - 198, In get_intermediate_data, the guards incorrectly check the built-in id instead of the refactored query parameter; replace the id checks so that when type == "global" you raise via flame_logger.raise_error if query is None, and when type == "local" you raise if both query is None and tag is None; also ensure the final non-tag branch fails fast if query is None before calling _get_file(f"/{type}/{query}"). Use the function name get_intermediate_data, parameters query and tag, and flame_logger.raise_error/new_log to locate and update the checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flamesdk/flame_core.py`:
- Around line 664-668: The code currently treats any non-EXECUTED status as
success, causing STOPPED/FAILED runs to get progress=100 and a "Node finished
successfully" log; update the guard around the success path in flame_core.py to
only run for non-terminal successful completion by checking that
self._flame_logger.runstatus is not in the terminal set (use
AnalysisStatus.EXECUTED.value, AnalysisStatus.STOPPED.value,
AnalysisStatus.FAILED.value) before calling set_progress,
_flame_logger.set_runstatus, flame_log, and config.finish_analysis so
aborted/failed analyses are not marked as successful.
In `@flamesdk/resources/client_apis/clients/message_broker_client.py`:
- Around line 201-207: The code only adds message IDs to
list_of_known_message_ids for non-local senders, so messages originated locally
can be redelivered and appended multiple times; update the handling in the
message receipt path (use the symbols message.body['meta']['sender'],
self.nodeConfig.node_id, self.list_of_known_message_ids,
self.list_of_incoming_messages, and self.flame_logger.new_log) so that you
always add message.body["meta"]["id"] to self.list_of_known_message_ids before
appending to self.list_of_incoming_messages; keep the existing conditional
logging for non-local messages but move the deduplication
(self.list_of_known_message_ids.add(...)) outside or into an else branch that
still marks local messages as known.
In `@flamesdk/resources/client_apis/clients/storage_client.py`:
- Around line 133-144: The generated filename uses
EXT_TO_OUTPUT_TYPE[output_type] even when local_dp is enabled and file_body is
actually UTF-8 text; change the logic so the chosen extension reflects the
actual wire format: compute an effective_output_type (e.g., set
effective_output_type = output_type normally, but if local_dp is true and
file_body is UTF-8-decodable text then set effective_output_type to the
textual/wire format you expect such as "json" or "text"), then use
EXT_TO_OUTPUT_TYPE[effective_output_type][0] when building resolved_name; update
the code paths around filename/resolved_name and references to
output_type/file_body in push_result (and the local_dp branch) so the uploaded
file extension matches the real wire format.
In `@flamesdk/resources/client_apis/message_broker_api.py`:
- Around line 91-97: The timeout check in the loop uses time_passed.seconds
which discards fractional seconds; update the condition in message_broker_api.py
(the loop computing time_passed = datetime.now() - start_time and using
acknowledged/receivers) to use time_passed.total_seconds() and compare with >=
timeout (and keep the existing break when counts match). Ensure the logged time
string (the flame_logger.new_log call that reports time_passed.microseconds) is
left as-is or adjusted separately, but the timeout enforcement must use
total_seconds() >= timeout for precise behavior.
In `@flamesdk/resources/client_apis/storage_api.py`:
- Around line 35-40: Do not overwrite the caller-provided multiple_results flag;
compute a separate derived flag for execution. Keep the original
multiple_results variable intact, compute result_len = len(result) if
multiple_results and isinstance(result, (list, tuple)) else 1, then set a new
local variable (e.g., actual_multiple = result_len != 1) and pass
actual_multiple as the multiple_result argument to
self._check_multi_result_validity(...) while leaving output_type and filename
handling unchanged so warnings for caller-requested multiple_results still fire.
In `@flamesdk/resources/rest_api.py`:
- Around line 59-63: The constructor leaves self.status_sync as None which
causes membership checks in apply_partner_status_to_self to raise; in the class
__init__ initialize/normalize self.status_sync to an empty set or list (e.g.,
set()) when the provided status_sync is None, and if you expect
iterable/deduplicated membership use a set; update any code that assumes a
specific type so apply_partner_status_to_self, status_sync, and any callers
consistently use that collection type.
In `@flamesdk/resources/utils/logging.py`:
- Around line 147-153: The raise_error method currently blocks by sleeping 1000
seconds by default; change its signature and behavior so it does not sleep by
default (e.g., set seconds=0 or remove the sleep entirely) and only sleep when
an explicit non-zero seconds is passed; update raise_error (and any callers like
FlameCoreSDK.flame_log) to rely on immediate logging via set_runstatus and
new_log without a long blocking time.sleep call. Ensure raise_error,
set_runstatus, and new_log remain called in the same order and preserve error
logging semantics, but eliminate the 1000-second default delay.
---
Outside diff comments:
In `@flamesdk/resources/client_apis/clients/data_api_client.py`:
- Around line 63-68: The S3 GET call is made outside the exception handler so
ConnectError/TimeoutException escape the except; move the
asyncio.run(self.client.get(...)) invocation into the same try block that calls
response.raise_for_status() (the block handling HTTPStatusError, ConnectError,
TimeoutException) so transient S3 failures are caught consistently; mirror the
pattern used for the FHIR path and _get_s3_dataset_names by wrapping the request
and subsequent response.raise_for_status() together inside the try surrounding
those exception types (update the block around the S3 request in
data_api_client.py where response is assigned).
In `@flamesdk/resources/client_apis/clients/storage_client.py`:
- Around line 159-198: In get_intermediate_data, the guards incorrectly check
the built-in id instead of the refactored query parameter; replace the id checks
so that when type == "global" you raise via flame_logger.raise_error if query is
None, and when type == "local" you raise if both query is None and tag is None;
also ensure the final non-tag branch fails fast if query is None before calling
_get_file(f"/{type}/{query}"). Use the function name get_intermediate_data,
parameters query and tag, and flame_logger.raise_error/new_log to locate and
update the checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 619cf84a-c531-4399-ab2a-5961f398a781
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
flamesdk/flame_core.pyflamesdk/resources/client_apis/clients/data_api_client.pyflamesdk/resources/client_apis/clients/message_broker_client.pyflamesdk/resources/client_apis/clients/po_client.pyflamesdk/resources/client_apis/clients/storage_client.pyflamesdk/resources/client_apis/message_broker_api.pyflamesdk/resources/client_apis/po_api.pyflamesdk/resources/client_apis/storage_api.pyflamesdk/resources/rest_api.pyflamesdk/resources/utils/constants.pyflamesdk/resources/utils/fhir.pyflamesdk/resources/utils/logging.pypyproject.toml
Comment on lines
+664
to
+668
| if self._flame_logger.runstatus != AnalysisStatus.EXECUTED.value: | ||
| self.set_progress(100) | ||
| self._flame_logger.set_runstatus(AnalysisStatus.EXECUTED.value) | ||
| self.flame_log("Node finished successfully") | ||
| self.config.finish_analysis() |
There was a problem hiding this comment.
Don’t run the success path for STOPPED/FAILED runs.
With the new terminal-state guard in FlameLogger.set_runstatus(), this block no longer changes the status, but it still sets progress to 100 and logs "Node finished successfully" for aborted runs. That makes stopped/failed analyses look successful.
💡 Suggested fix
- if self._flame_logger.runstatus != AnalysisStatus.EXECUTED.value:
+ if self._flame_logger.runstatus not in {
+ AnalysisStatus.EXECUTED.value,
+ AnalysisStatus.STOPPED.value,
+ AnalysisStatus.FAILED.value,
+ }:
self.set_progress(100)
self._flame_logger.set_runstatus(AnalysisStatus.EXECUTED.value)
self.flame_log("Node finished successfully")
- self.config.finish_analysis()
+ if not self.config.finished:
+ self.config.finish_analysis()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flamesdk/flame_core.py` around lines 664 - 668, The code currently treats any
non-EXECUTED status as success, causing STOPPED/FAILED runs to get progress=100
and a "Node finished successfully" log; update the guard around the success path
in flame_core.py to only run for non-terminal successful completion by checking
that self._flame_logger.runstatus is not in the terminal set (use
AnalysisStatus.EXECUTED.value, AnalysisStatus.STOPPED.value,
AnalysisStatus.FAILED.value) before calling set_progress,
_flame_logger.set_runstatus, flame_log, and config.finish_analysis so
aborted/failed analyses are not marked as successful.
Comment on lines
+201
to
207
| if message.body['meta']['sender'] != self.nodeConfig.node_id: | ||
| self.flame_logger.new_log(f"received message from {message.body['meta']['sender']}", | ||
| log_type=LogTypeLiteral.INFO.value) | ||
| self.flame_logger.new_log(f"message body: {message.body}", | ||
| log_type=LogTypeLiteral.DEBUG.value) | ||
| self.list_of_known_message_ids.add(message.body["meta"]["id"]) | ||
| self.list_of_incoming_messages.append(message) |
There was a problem hiding this comment.
Local-sender deliveries are no longer deduplicated.
Lines 201-206 only mark non-local messages as known. A redelivered acknowledgement will keep is_new_message == True, so the same message can be appended multiple times and later treated as fresh traffic.
Proposed fix
if is_new_message:
if message.body['meta']['sender'] != self.nodeConfig.node_id:
self.flame_logger.new_log(f"received message from {message.body['meta']['sender']}",
log_type=LogTypeLiteral.INFO.value)
self.flame_logger.new_log(f"message body: {message.body}",
log_type=LogTypeLiteral.DEBUG.value)
- self.list_of_known_message_ids.add(message.body["meta"]["id"])
+ self.list_of_known_message_ids.add(message.body["meta"]["id"])
self.list_of_incoming_messages.append(message)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flamesdk/resources/client_apis/clients/message_broker_client.py` around lines
201 - 207, The code only adds message IDs to list_of_known_message_ids for
non-local senders, so messages originated locally can be redelivered and
appended multiple times; update the handling in the message receipt path (use
the symbols message.body['meta']['sender'], self.nodeConfig.node_id,
self.list_of_known_message_ids, self.list_of_incoming_messages, and
self.flame_logger.new_log) so that you always add message.body["meta"]["id"] to
self.list_of_known_message_ids before appending to
self.list_of_incoming_messages; keep the existing conditional logging for
non-local messages but move the deduplication
(self.list_of_known_message_ids.add(...)) outside or into an else branch that
still marks local messages as known.
Comment on lines
+91
to
97
| time_passed = datetime.now() - start_time | ||
| self.message_broker_client.flame_logger.new_log( | ||
| f"{len(acknowledged)} acknowledged (time={time_passed.microseconds}{chr(956)}s)", | ||
| log_type=LogTypeLiteral.DEBUG.value | ||
| ) | ||
| if (len(acknowledged) == len(receivers)) or ((timeout is not None) and (time_passed.seconds > timeout)): | ||
| break |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current timeout comparison usage in the changed file.
rg -nP 'time_passed\.seconds\s*>\s*timeout|time_passed\.microseconds' flamesdk/resources/client_apis/message_broker_api.py
# Verify Python timedelta behavior relevant to this condition.
python - <<'PY'
from datetime import timedelta
d = timedelta(seconds=10, microseconds=500000)
print("timedelta.seconds =", d.seconds) # integer seconds component only
print("timedelta.total_seconds() =", d.total_seconds()) # precise elapsed seconds
PYRepository: PrivateAIM/python-sdk
Length of output: 342
Use total_seconds() for timeout enforcement
Line 96 uses time_passed.seconds > timeout, which returns only the integer seconds component and can delay timeout handling by truncating fractional seconds. Use total_seconds() with >= for precise timeout behavior.
Proposed fix
- time_passed = datetime.now() - start_time
+ time_passed = datetime.now() - start_time
+ elapsed_seconds = time_passed.total_seconds()
self.message_broker_client.flame_logger.new_log(
- f"{len(acknowledged)} acknowledged (time={time_passed.microseconds}{chr(956)}s)",
+ f"{len(acknowledged)} acknowledged (time={elapsed_seconds:.6f}s)",
log_type=LogTypeLiteral.DEBUG.value
)
- if (len(acknowledged) == len(receivers)) or ((timeout is not None) and (time_passed.seconds > timeout)):
+ if (len(acknowledged) == len(receivers)) or ((timeout is not None) and (elapsed_seconds >= timeout)):
break📝 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
| time_passed = datetime.now() - start_time | |
| self.message_broker_client.flame_logger.new_log( | |
| f"{len(acknowledged)} acknowledged (time={time_passed.microseconds}{chr(956)}s)", | |
| log_type=LogTypeLiteral.DEBUG.value | |
| ) | |
| if (len(acknowledged) == len(receivers)) or ((timeout is not None) and (time_passed.seconds > timeout)): | |
| break | |
| time_passed = datetime.now() - start_time | |
| elapsed_seconds = time_passed.total_seconds() | |
| self.message_broker_client.flame_logger.new_log( | |
| f"{len(acknowledged)} acknowledged (time={elapsed_seconds:.6f}s)", | |
| log_type=LogTypeLiteral.DEBUG.value | |
| ) | |
| if (len(acknowledged) == len(receivers)) or ((timeout is not None) and (elapsed_seconds >= timeout)): | |
| break |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flamesdk/resources/client_apis/message_broker_api.py` around lines 91 - 97,
The timeout check in the loop uses time_passed.seconds which discards fractional
seconds; update the condition in message_broker_api.py (the loop computing
time_passed = datetime.now() - start_time and using acknowledged/receivers) to
use time_passed.total_seconds() and compare with >= timeout (and keep the
existing break when counts match). Ensure the logged time string (the
flame_logger.new_log call that reports time_passed.microseconds) is left as-is
or adjusted separately, but the timeout enforcement must use total_seconds() >=
timeout for precise behavior.
Comment on lines
+35
to
+40
| result_len = len(result) if multiple_results and isinstance(result, (list, tuple)) else 1 | ||
| multiple_results = result_len != 1 | ||
| output_type, filename = self._check_multi_result_validity(multiple_result=multiple_results, | ||
| output_type=output_type, | ||
| filename=filename, | ||
| result_length=result_len) |
There was a problem hiding this comment.
Preserve the caller’s multiple_results intent.
Lines 35-36 overwrite multiple_results based on result_len, so submit_final_result([item], multiple_results=True) now falls into the single-result path and uploads the whole list as one artifact. It also prevents the warning on Lines 66-70 from ever firing for multiple_results=True with non-sequence inputs. Keep the requested flag separate from the derived execution path.
Suggested fix
- result_len = len(result) if multiple_results and isinstance(result, (list, tuple)) else 1
- multiple_results = result_len != 1
- output_type, filename = self._check_multi_result_validity(multiple_result=multiple_results,
+ requested_multiple_results = multiple_results
+ effective_multiple_results = requested_multiple_results and isinstance(result, (list, tuple))
+ result_len = len(result) if effective_multiple_results else 1
+ output_type, filename = self._check_multi_result_validity(multiple_result=effective_multiple_results,
output_type=output_type,
filename=filename,
result_length=result_len)
@@
- if multiple_results and isinstance(result, (list, tuple)):
+ if effective_multiple_results:
@@
- if multiple_results:
+ if requested_multiple_results and not isinstance(result, (list, tuple)):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flamesdk/resources/client_apis/storage_api.py` around lines 35 - 40, Do not
overwrite the caller-provided multiple_results flag; compute a separate derived
flag for execution. Keep the original multiple_results variable intact, compute
result_len = len(result) if multiple_results and isinstance(result, (list,
tuple)) else 1, then set a new local variable (e.g., actual_multiple =
result_len != 1) and pass actual_multiple as the multiple_result argument to
self._check_multi_result_validity(...) while leaving output_type and filename
handling unchanged so warnings for caller-requested multiple_results still fire.
Comment on lines
+147
to
153
| def raise_error(self, message: str, seconds: int = 1000) -> None: | ||
| if self.runstatus not in [AnalysisStatus.EXECUTED.value, | ||
| AnalysisStatus.STOPPED.value, | ||
| AnalysisStatus.FAILED.value]: | ||
| self.set_runstatus(AnalysisStatus.FAILED.value) | ||
| self.new_log(message, log_type=LogTypeLiteral.ERROR.value) | ||
| time.sleep(seconds) |
There was a problem hiding this comment.
Remove the 1000-second default sleep from raise_error.
FlameCoreSDK.flame_log() sends every ERROR log through this method, and the REST endpoints call it before returning failures. With the new default, routine startup/API errors now block for ~16 minutes instead of surfacing immediately.
💡 Suggested fix
- def raise_error(self, message: str, seconds: int = 1000) -> None:
+ def raise_error(self, message: str, seconds: int = 0) -> None:
if self.runstatus not in [AnalysisStatus.EXECUTED.value,
AnalysisStatus.STOPPED.value,
AnalysisStatus.FAILED.value]:
self.set_runstatus(AnalysisStatus.FAILED.value)
self.new_log(message, log_type=LogTypeLiteral.ERROR.value)
- time.sleep(seconds)
+ if seconds > 0:
+ time.sleep(seconds)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flamesdk/resources/utils/logging.py` around lines 147 - 153, The raise_error
method currently blocks by sleeping 1000 seconds by default; change its
signature and behavior so it does not sleep by default (e.g., set seconds=0 or
remove the sleep entirely) and only sleep when an explicit non-zero seconds is
passed; update raise_error (and any callers like FlameCoreSDK.flame_log) to rely
on immediate logging via set_runstatus and new_log without a long blocking
time.sleep call. Ensure raise_error, set_runstatus, and new_log remain called in
the same order and preserve error logging semantics, but eliminate the
1000-second default delay.
antidodo
changed the title
Co-authored-by: antidodo <albin2993@gmail.com>
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
2 participants
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 v0.6.0
New Features
Bug Fixes
Improvements