Skip to content
Snippets Groups Projects

Add preliminary smart container list, throws a warning not an exception

Merged Dan Guest requested to merge dguest/athena:add-prelim-smart-containers into 21.2

This is a fix for !25104 (merged), which broke several ART tests.

The problem was that we added a new smart collection for VR track jets, which everyone used to add to AllVariables. Of course AllVariables is disallowed for collections with smart collections (at least in some derivations), so adding a smart collection broke several of our bigger derivations.

With this MR, the SlimmerHelper will check for the offending collection in PreliminarySmartCollections. If the collection is listed as a "preliminary" smart collection it will just throw a warning.

I think this solution is the least evil of several options:

  • I can't validate every derivation to make sure this smart collection works for them. This is something that will have to be up to the derivations clients.
  • I don't want to break anyone's derivation, and unfortunately several derivations are broken right now.
  • I don't want to keep adding VR track jets to AllVariables: most people need very few of the variables we're saving there.

I'm guessing some people would opt for a more draconian approach where we simply leave the derivations broken and leave it up to the derivation contacts to fix them. That sort of decision seems above my pay grade and beyond the timescale where we'd want to have the ART tests fixed (maybe a conversation in the derivation coordination meeting would be appropriate at some point).

Merge request reports

Pipeline #1006379 passed

Pipeline passed for 9a62a889 on dguest:add-prelim-smart-containers

Approval is optional

Merged by Oana Vickey BoeriuOana Vickey Boeriu 5 years ago (Aug 4, 2019 11:45am UTC)

Merge details

  • Changes merged into 21.2 with 6431c765.
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
Please register or sign in to reply
Loading