Skip to content

Conversation

@chrisburr
Copy link
Member

@chrisburr chrisburr commented Jan 29, 2026

Summary

  • Return error dictionaries with errno codes from exists() and getFileMetadata() in GFAL2_StorageBase
  • Captures gfal2.GError separately to extract the e.code (errno)
  • Allows consumers to distinguish between different error types (e.g., ENOENT vs EACCES)

See: https://gitlab.cern.ch/lhcb-dirac/LHCbDIRAC/-/merge_requests/1870

Motivation

When a storage element returns HTTP 401 (Permission Denied / Authentication Error), dirac-dms-check-fc2se incorrectly reports files as "missing at SEs". This is because all files in metadata["Failed"] are treated as "not existing" regardless of the actual error type.

By returning structured error information with errno codes, downstream code can properly handle permission denied errors separately from file-not-found errors, avoiding false positives.

Changes

The error format changes from:

failed[url] = repr(e)  # string

to:

failed[url] = {"error": repr(e), "errno": e.code}  # dict with errno

Backward Compatibility

Downstream code should handle both the old string format and new dict format during the transition period:

if isinstance(err_info, dict):
    err_code = err_info.get("errno")
else:
    err_code = None  # Legacy string format

Return error dictionaries with errno codes from exists() and
getFileMetadata() to allow consumers to distinguish between different
error types (e.g., ENOENT vs EACCES).

This enables downstream code to properly handle permission denied errors
separately from file-not-found errors, avoiding false positives when
checking file existence at storage elements.
@chrisburr chrisburr marked this pull request as ready for review January 29, 2026 05:38
@chrisburr chrisburr requested a review from chaen January 29, 2026 08:52
@fstagni fstagni merged commit dfc2432 into DIRACGrid:integration Jan 29, 2026
23 checks passed
@DIRACGridBot DIRACGridBot added the sweep:ignore Prevent sweeping from being ran for this PR label Jan 29, 2026
@chaen
Copy link
Contributor

chaen commented Jan 29, 2026

Ouuuuh dangerous !

@chaen
Copy link
Contributor

chaen commented Jan 29, 2026

We tried such things in the past and it was a disaster because upper in the chain we do string comparisons which of course break against the dictionary

@chaen
Copy link
Contributor

chaen commented Jan 29, 2026

that was merged too quickly, it needs to be carefully checked

@chaen
Copy link
Contributor

chaen commented Jan 29, 2026

just to add, I am happy with the idea, but we never converged on a way to do it. Maybe time to rethink it globally.

@chaen
Copy link
Contributor

chaen commented Jan 29, 2026

Btw that's why DErrno.cmpError also can take a string as input. It was to provide a migration path. It should work for he LHCbDIRAC MR I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sweep:ignore Prevent sweeping from being ran for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants