Skip to content

Conversation

Copy link

Copilot AI commented Nov 14, 2025

Description

The daliadmin set command prints "Changed" regardless of whether the operation succeeds or fails. When connection to the Dali store fails, both an error and a success message are printed:

$ ./daliadmin . set '/Files/Scope[name="qa"]/.../File/@name' value
ERROR: Could not connect to /Files/Scope[name="qa"]/.../File/@name
Changed /Files/Scope[name="qa"]/.../File/@name from '' to 'value'

Changes

Modified setValue() function signature

  • Return type changed from StringBuffer& to bool
  • Returns false on connection failure, true on success
  • Updated in daadmin.cpp and daadmin.hpp

Updated caller in daliadmin.cpp

  • Check return value before printing success message
  • Error message still prints via UERRLOG on failure

No other code references this function.

Type of change:

  • This change is a bug fix (non-breaking change which fixes an issue).
  • This change is a new feature (non-breaking change which adds functionality).
  • This change improves the code (refactor or other change that does not change the functionality)
  • This change fixes warnings (the fix does not alter the functionality or the generated code)
  • This change is a breaking change (fix or feature that will cause existing behavior to change).
  • This change alters the query API (existing queries will have to be recompiled)

Checklist:

  • My code follows the code style of this project.
    • My code does not create any new warnings from compiler, build system, or lint.
  • The commit message is properly formatted and free of typos.
    • The commit message title makes sense in a changelog, by itself.
    • The commit is signed.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly, or...
    • I have created a JIRA ticket to update the documentation.
    • Any new interfaces or exported functions are appropriately commented.
  • I have read the CONTRIBUTORS document.
  • The change has been fully tested:
    • I have added tests to cover my changes.
    • All new and existing tests passed.
    • I have checked that this change does not introduce memory leaks.
    • I have used Valgrind or similar tools to check for potential issues.
  • I have given due consideration to all of the following potential concerns:
    • Scalability
    • Performance
    • Security
    • Thread-safety
    • Cloud-compatibility
    • Premature optimization
    • Existing deployed queries will not be broken
    • This change fixes the problem, not just the symptom
    • The target branch of this pull request is appropriate for such a change.
  • There are no similar instances of the same problem that should be addressed
    • I have addressed them here
    • I have raised JIRA issues to address them separately
  • This is a user interface / front-end modification
    • I have tested my changes in multiple modern browsers
    • The component(s) render as expected

Smoketest:

  • Send notifications about my Pull Request position in Smoketest queue.
  • Test my draft Pull Request.

Testing:

Manual verification of code changes. The fix follows the same pattern used by other daliadmin commands (importFromFile, erase, add) which already return bool for success/failure.

Original prompt

There's a bug in the export function daliadmin (code is in dali/daliadmin/daadmin.cpp), it reports "Changed" regardless of whether the command succeeds or fails. It should only output this on success.

Here is an example:

$ ./daliadmin . set '/Files/Scope[name="qa"]/Scope[name="tracesmart"]/Scope[name="key"]/Scope[name="death_combined"]/File/@name' payloadbyuklexid
ERROR: Could not connect to /Files/Scope[name="qa"]/Scope[name="tracesmart"]/Scope[name="key"]/Scope[name="death_combined"]/File/@name
Changed /Files/Scope[name="qa"]/Scope[name="tracesmart"]/Scope[name="key"]/Scope[name="death_combined"]/File/@name from '' to 'payloadbyuklexid'
dave@RISKAZ-06905526:~/hpccDebug/opt/HPCCSystems/bin

Fix this issue


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Modified setValue function to return bool instead of StringBuffer& to indicate success/failure.
Updated caller to check return value before printing success message.
This fixes the bug where 'Changed' was printed even when connection failed.

Co-authored-by: jakesmith <[email protected]>
Copilot AI changed the title [WIP] Fix bug in export function of daliadmin Fix daliadmin set command reporting "Changed" on connection failure Nov 14, 2025
Copilot AI requested a review from jakesmith November 14, 2025 10:09
Copilot finished work on behalf of jakesmith November 14, 2025 10:09
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.

2 participants