Skip to content

fix: wrap DCM project name in IDENTIFIER() for list/drop deployments#2997

Draft
sfc-gh-olorek wants to merge 1 commit into
mainfrom
proactive/dcm-sql-identifier-consistency
Draft

fix: wrap DCM project name in IDENTIFIER() for list/drop deployments#2997
sfc-gh-olorek wants to merge 1 commit into
mainfrom
proactive/dcm-sql-identifier-consistency

Conversation

@sfc-gh-olorek
Copy link
Copy Markdown
Contributor

Pre-review checklist

  • I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
  • I've added or updated automated unit tests to verify correctness of my new code.
  • I've added or updated integration tests to verify correctness of my new code.
  • I've confirmed that my changes are working by executing CLI's commands manually on MacOS.
  • I've confirmed that my changes are working by executing CLI's commands manually on Windows.
  • I've confirmed that my changes are up-to-date with the target branch.
  • I've described my changes in the release notes.
  • I've described my changes in the section below.
  • I've described my changes in the documentation.

Changes description

DCMProjectManager emits SQL for 10 DCM subcommands, but the project-name identifier was being injected two different ways:

  • 8 methods use FQN.sql_identifier, which wraps the name in IDENTIFIER('<name>'). This is the conventional pattern across the CLI (see apps/manager.py's ALTER WORKSPACE {workspace_fqn.sql_identifier} call sites) and is the form used everywhere else in the DCM plugin: EXECUTE DCM PROJECT IDENTIFIER('<name>') DEPLOY/ANALYZE/PLAN/PREVIEW/REFRESH/PURGE/TEST, and CREATE DCM PROJECT IDENTIFIER('<name>').
  • 2 methods — list_deployments and drop_deployment — were still using the raw FQN.identifier property, emitting SHOW DEPLOYMENTS IN DCM PROJECT <name> and ALTER DCM PROJECT <name> DROP DEPLOYMENT.

That inconsistency meant these two subcommands did not normalize fully-qualified / case-sensitive project names through the same IDENTIFIER(...) path that the rest of the manager (and the account-level session) relies on, producing different behaviour for the same FQN depending on which snow dcm subcommand the user ran.

This PR makes both call sites use project_identifier.sql_identifier, matching the other 8 sites and the parser-supported form (SHOW ... IN DCM PROJECT IDENTIFIER(:PROJ_NAME), ALTER DCM PROJECT <id> DROP DEPLOYMENT).

Testing

  • Updated the two affected unit tests in tests/dcm/test_manager.py to assert on the new IDENTIFIER('my_project') wrapping.
  • Full DCM unit suite (tests/dcm/) passes locally — 295 tests, 46 snapshots.

`snow dcm list-deployments` and `snow dcm drop-deployment` emitted
`SHOW DEPLOYMENTS IN DCM PROJECT <name>` and
`ALTER DCM PROJECT <name> DROP DEPLOYMENT` using the raw
`FQN.identifier` property, while every other DCM subcommand in
`DCMProjectManager` already wraps the name with `FQN.sql_identifier`
(i.e. `IDENTIFIER('<name>')`).

This inconsistency caused fully-qualified or case-sensitive project
names to parse incorrectly for exactly these two subcommands. Using
the same `sql_identifier` wrapping everywhere in the manager brings
the two stragglers in line with the remaining 8 call sites and with
the established pattern used in the apps/workspace plugins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant