Fix: handle duplicate key error when we add a project var or change the project name

[lafirm]

Should fix #4497

I found a bug while attempting to convert a monorepo to a multi-repo setup by adding a project variable in the config YAML. Upon investigation, I discovered that the same error also occurs when attempting to rename a project.

The root cause appears to be a line of code that tries to update the _models variable, which is a UniqueKeyDict that already contains the project models. It raises the Duplicatekey error when the models are re-added.

To address this, I’ve submitted a simple and humble PR that assumes we can safely skip adding models if they already exist. I’d appreciate it if you could review the changes and share your thoughts.

_

Additionally, I have a request: would it be possible to release a bugfix version 0.174.3 with this patch? We’re currently unable to upgrade sqlmesh beyond 0.174.* for the time being, but we do need this fix.

_

Thanks for your time and support!

[izeigerman]

@lafirm thanks a lot for your contribution!

I'm a little concerned that this fix may mask a real issue. The idea there is that models sourced from the prod environment should not overlap with models loaded locally. So we should, under no circumstance, override the local version of the model with the one sourced from prod. Otherwise, user changes could get lost.

@georgesittas @tobymao wdyt? Is there a better approach to fixing this?

[themisvaltinos]

I'm a little concerned that this fix may mask a real issue. The idea there is that models sourced from the prod environment should not overlap with models loaded locally. So we should, under no circumstance, override the local version of the model with the one sourced from prod. Otherwise, user changes could get lost.

@izeigerman looking into it I can't think there’s a scenario where a production model silently overrides a local one as even if it did store[snapshot.name] = snapshot.node would raise if the same key already exists, so a local model can’t be overwritten.

I was thinking if there is a concern of models that exist in prod but not locally (e.g. one living only in repo_2) will be pulled in the models, but that is a desired behaviour.

Also if the model exists both locally and in prod, it’s flagged in uncached but not updated with the prod version.

if there is an edge case i can't think of @lafirm can also restructure this to make it more explicit maybe:

local_store = self._standalone_audits if snapshot.is_audit else self._models
if snapshot.name in local_store:
    uncached.add(snapshot.name)
else:
    local_store[snapshot.name] = snapshot.node