Make collocation group optional

[zachlefevre]

If collocation group is not given then default to empty set

[catstavi]

Code makes sense and does what the PR title says! Since I'm not very familiar with this area, could you help grow my knowledge and explain what the "group" parameter signifies in a CollocationConfig, and why we want it to be optional?

[zachlefevre]

@catstavi i'm not sure! I do know that this config value is set in secondary-watcher-pg even though it's unused. All over the place we're setting this env variable for those services, depiste it being unnecessary to set it for those services.

Therefore I can do 3 things. 1) make it a [] in the secondary-watcher-pg. 2) make it optional here so it's default is {}. 3) make secondary-watcher-pg use a different config than this.

3 is more work than this is worth and I flipped a coin between 1 and 2.

[catstavi]

Summary of discussion:

• CollocationConfig is a shared model between data-coordinator and the various things that interact with secondaries (secondary watchers, archival stuff, etc). The secondary-things import it
• However, data-coordinator is the only user that needs to know anything about collocation groups. This information is not used, and not relevant on an individual secondary level
• But because to create the config object, you need to provide the 'group' attribute, we've been copying + updating and environment variable that lists the secondaries in all of the apps-marathon files for secondary-watcher, and its a pain. Its also distracting, meaningless clutter.
• Cons of this approach is that people coding in the secondary context may assume that the 'groups' property on their CollocationConfig object is meaningful, and nothing in the type system tells them that its not (will always be empty).
• we agreed that the best solution would be for data-coordinator and the secondaries should really being using different objects that represent the different sets of meaningful data; or to make the 'group' an Option so that the type system can indicate its lack of presence.
  • however, this would be a much larger changeset touching across multiple secondaries, and likely not end up getting implemented; the fix as is saves us the pain and clutter on the apps-marathon size. Perfect must not be the enemy of the good.

TODO: Zach is going to write a comment in the CollocationConfig to explain why that parameter is considered optional (not used in the context of the secondary applications)

[catstavi]

@zachlefevre add a comment so I can approve this and you can merge!