digger icon indicating copy to clipboard operation
digger copied to clipboard

'dependency_configuration' is undocumented and possibly has wrong default behaviour

Open ahuseby opened this issue 11 months ago • 3 comments

Use of dependency_configuration is evidently supported, but lacks documentation in the Digger user documentation. I'm not sure if https://github.com/diggerhq/digger/issues/782 is supposed to address this.

Furthermore dependency_configuration could potetially have the wrong default behaviour, based on the definition of the two available modes (hard and soft). https://github.com/diggerhq/digger/blob/d9ac240301220fd5aa3cc0552f0afcb8530b0975/libs/digger_config/converters.go#L12-L17

My experience is that without specifying anything for dependency_configuration.mode, the following happens

We have 2 projects, A and B B is dependent on A (using 'depends_on')

We want to make a change that only affects B

  1. If we commit a change that only affects B, nothing happens.
  2. If we then add a commit with change affecting A, then a plan run starts for A and B.

The way I observe this, in regards to adjusting dependency_configuration and how it currently functions

  • The hard (default) mode requires all projects in a dependency chain to become affected by change.
  • The soft mode will e.g. allow a change only to B. This is what we want.

However, the definition from the source code tell us the opposite.

// soft - if dependency project wasn't changed, it will be skipped

So seemingly the actual observed default behaviour is that of "soft", by definition.

Is the issue perhaps on this line in the orchestrator code? https://github.com/diggerhq/digger/blob/d9ac240301220fd5aa3cc0552f0afcb8530b0975/libs/orchestrator/github/github.go#L476 Perhaps change the condition to diggerConfig.DependencyConfiguration.Mode == digger_config.DependencyConfigurationSoft?

I have discussed this with and demonstrated it on video call to @motatoes. In those discussions I've described a transitive dependency C -> B -> A, but that doesn't seem to matter at all. With the default behaviour, for C to plan, I have to make a change affecting A aswell.

ahuseby avatar Mar 08 '24 14:03 ahuseby

I'm happy to contribute changes, but first need to clarify what the definition of DependencyConfiguration.Mode is and what the default behaviour is expected to be 🙂

ahuseby avatar Mar 08 '24 14:03 ahuseby

I've discussed with @Spartakovic regarding this and he said that there might be a bug indeed with the "hard" behaviour and asked me to try and replace the line https://github.com/diggerhq/digger/blob/d9ac240301220fd5aa3cc0552f0afcb8530b0975/libs/orchestrator/github/github.go#L520 to true might be the fix. Going to further investigate

motatoes avatar Mar 08 '24 14:03 motatoes

Please let me know if you need someone to test this :)

ahuseby avatar Mar 11 '24 09:03 ahuseby