Skip to content

fix(): S3 compat for on-prem (s3proxy/MinIO) and graceful empty-log handling#1893

Merged
benflexcompute merged 5 commits intomainfrom
fix/nexus-s3proxy-compat
Mar 14, 2026
Merged

fix(): S3 compat for on-prem (s3proxy/MinIO) and graceful empty-log handling#1893
benflexcompute merged 5 commits intomainfrom
fix/nexus-s3proxy-compat

Conversation

@MatthewSwords
Copy link
Contributor

@MatthewSwords MatthewSwords commented Mar 13, 2026

Summary

  • S3 checksum compatibility: When s3_endpoint_url is set (Nexus/on-prem deployments), disable default boto3 checksum headers (x-amz-checksum-*) via BotocoreConfig(request_checksum_calculation="when_required"). s3proxy does not implement these headers, causing PutObject to fail with NotImplemented. This is a no-op for production (AWS S3) since the setting only activates when a custom endpoint is configured.
  • Graceful empty-log handling: case.logs.tail() on a job that never produced solver logs (e.g. meshing failed before solver ran) threw IndexError from an unchecked [0] on an empty list. Now raises FileNotFoundError with a clear message.

Context

Discovered during Nexus dev setup testing with the flow360 Python client. The S3 issue affects all file uploads when using s3proxy (Nexus dev) or certain MinIO versions (Nexus release packages). The log fix is a minor edge case for errored jobs.

Test plan

  • Verified S3 upload works against s3proxy with this fix (geometry upload succeeds)
  • Verified case._download_file() still works for file downloads
  • Verified case.logs.tail() now gives FileNotFoundError instead of IndexError when no logs exist
  • Verify production (AWS S3) uploads still work (no behavior change when s3_endpoint_url is None)

Made with Cursor


Note

Medium Risk
Changes S3 client configuration (checksums and addressing style) when using a custom endpoint, which could affect upload/download behavior in on-prem deployments. Also changes log retrieval to raise a clearer runtime error when no log files exist, altering error behavior for some failed jobs.

Overview
Improves S3 compatibility for on-prem/S3-compatible backends by customizing BotocoreConfig when Env.current.s3_endpoint_url is set: disables default checksum headers, forces path-style addressing, and falls back gracefully on older botocore versions.

Updates RemoteResourceLogs._get_tmp_file_name() to detect when no .log files are available and raise Flow360RuntimeError with a clear message instead of failing via list indexing; adds tests covering empty and non-matching file lists.

Written by Cursor Bugbot for commit c5852ee. This will update automatically on new commits. Configure here.

…andling

Two fixes for Nexus (on-premises) deployments:

1. S3 checksum compatibility: newer boto3 versions send x-amz-checksum
   headers by default, which s3proxy and some MinIO versions do not
   implement — causing PutObject to fail with NotImplemented. When
   s3_endpoint_url is set (indicating a non-AWS S3 backend), disable
   default checksum calculation/validation via BotocoreConfig. This is
   a no-op for production (AWS S3) since the setting only activates
   when a custom endpoint is configured.

2. Log access on errored jobs: accessing case.logs.tail() on a job that
   never produced solver logs (e.g. meshing failed before solver ran)
   threw an IndexError from an unchecked [0] on an empty list. Now
   raises FileNotFoundError with a clear message instead.

Made-with: Cursor
Copilot AI review requested due to automatic review settings March 13, 2026 19:00
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Copy link
Contributor

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 improves the Flow360 Python client’s robustness in two areas that commonly affect on-prem/Nexus dev setups: S3-compatible storage uploads (e.g., s3proxy/MinIO) and handling resources that never produced solver logs.

Changes:

  • Adjust S3 client construction to disable default checksum header behavior when using a custom s3_endpoint_url (S3-compatible backends).
  • Make RemoteResourceLogs raise a clear FileNotFoundError when no log files exist instead of failing with an IndexError.

Reviewed changes

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

File Description
flow360/component/resource_base.py Adds explicit empty-log detection and raises FileNotFoundError with a clearer message.
flow360/cloud/s3_utils.py Extends botocore client config for custom S3 endpoints to avoid incompatible checksum headers/validation.
Comments suppressed due to low confidence (1)

flow360/cloud/s3_utils.py:169

  • There’s now behavior branching on Env.current.s3_endpoint_url to change checksum settings, but there are existing tests for flow360/cloud/s3_utils.py (e.g. tests/test_s3_utils.py) and none validate that get_client() passes the intended BotocoreConfig when a custom endpoint is set. Adding a unit test that monkeypatches boto3.client to capture the config argument would help prevent regressions in the on-prem compatibility fix.
        config_kwargs = {"max_pool_connections": MAX_POOL}
        if Env.current.s3_endpoint_url is not None:
            # S3-compatible stores (s3proxy, MinIO) may not implement the
            # checksum headers that newer boto3 versions send by default.
            config_kwargs["request_checksum_calculation"] = "when_required"
            config_kwargs["response_checksum_validation"] = "when_required"

        kwargs = {
            "region_name": self.user_credential.region,
            "aws_access_key_id": self.user_credential.access_key_id,
            "aws_secret_access_key": self.user_credential.secret_access_key,
            "aws_session_token": self.user_credential.session_token,
            "config": BotocoreConfig(**config_kwargs),
        }

        if Env.current.s3_endpoint_url is not None:
            kwargs["endpoint_url"] = Env.current.s3_endpoint_url

        return client("s3", **kwargs)

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

- Change FileNotFoundError → Flow360RuntimeError so the exception is not
  silently caught by the except OSError handlers in _get_log_by_pos and
  _get_log_by_level (FileNotFoundError is a subclass of OSError)
- Move log file check before TemporaryLogDirectory creation to avoid
  leaving an unused temp dir when no logs exist
- Wrap BotocoreConfig construction in try/except TypeError to gracefully
  fall back when older botocore versions don't support the checksum
  config options
- Add two tests for the empty-log scenario: empty file list and file
  list with no matching *.log entries

Made-with: Cursor
Made-with: Cursor
@benflexcompute benflexcompute changed the title fix(): S3 compat for on-prem (s3proxy/MinIO) and graceful empty-log handling [FXC-6257] fix(): S3 compat for on-prem (s3proxy/MinIO) and graceful empty-log handling Mar 13, 2026
@benflexcompute benflexcompute changed the title [FXC-6257] fix(): S3 compat for on-prem (s3proxy/MinIO) and graceful empty-log handling fix(): S3 compat for on-prem (s3proxy/MinIO) and graceful empty-log handling Mar 13, 2026
benflexcompute
benflexcompute previously approved these changes Mar 13, 2026
Virtual-hosted style (http://bucket.host/key) requires DNS resolution of
the bucket subdomain, which fails in Kubernetes and single-node on-prem
setups. Path-style (http://host/bucket/key) works universally with
s3proxy, MinIO, and real AWS S3. Only activated when s3_endpoint_url is
set (on-prem); production behavior unchanged.

Made-with: Cursor
Copy link
Contributor

@mikeparkflex mikeparkflex left a comment

Choose a reason for hiding this comment

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

My agent with context from debugging concurs. No open items remaining. Both PRs are ready to merge.

@benflexcompute benflexcompute enabled auto-merge (squash) March 14, 2026 15:31
@benflexcompute benflexcompute merged commit 0f487d4 into main Mar 14, 2026
20 checks passed
@benflexcompute benflexcompute deleted the fix/nexus-s3proxy-compat branch March 14, 2026 15:39
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.

4 participants