Don't fail on saveMetrics and saveLineage

[shnapz]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 61.47%. Comparing base (36db465) to head (0ed0cbf).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
...e/src/main/scala/com/spotify/scio/ScioResult.scala	85.71%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5718   +/-   ##
=======================================
  Coverage   61.46%   61.47%           
=======================================
  Files         314      314           
  Lines       11354    11358    +4     
  Branches      789      808   +19     
=======================================
+ Hits         6979     6982    +3     
- Misses       4375     4376    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[clairemcginty]

hmm, where is path getting used in this method outside of the logging? can we just log the resourceID directly?

[shnapz]

the problem is that creation of resourceId can fail if bucket does not exist

[clairemcginty]

I think that's ok; if we do:

      logger.info(f"Saved metrics to '$resourceId'")
    } catch {
      case e: Throwable =>
        logger.warn(
          f"Failed to save metrics: ${mapper.writeValueAsString(value)}",
          e
        )

in the success case, the resourceID exists so it's safe to log, and in the failure case, the error msg would include the path anyway, right?

[shnapz]

actually I re-tested it and resourceID is safely created even if bucket does not exist! So I have updated the code and replaced path with resourceId!

[clairemcginty]

I think that's ok; if we do:

      logger.info(f"Saved metrics to '$resourceId'")
    } catch {
      case e: Throwable =>
        logger.warn(
          f"Failed to save metrics: ${mapper.writeValueAsString(value)}",
          e
        )

in the success case, the resourceID exists so it's safe to log, and in the failure case, the error msg would include the path anyway, right?