Skip to content

dird: fix director resource not showing when using show director or the --xc director cli option#1315

Merged
joergsteffens merged 7 commits intobareos:masterfrom
alaaeddineelamri:dev/alaaeddineelamri/master/s5321-fix-show-directors
Nov 30, 2022
Merged

dird: fix director resource not showing when using show director or the --xc director cli option#1315
joergsteffens merged 7 commits intobareos:masterfrom
alaaeddineelamri:dev/alaaeddineelamri/master/s5321-fix-show-directors

Conversation

@alaaeddineelamri
Copy link
Contributor

@alaaeddineelamri alaaeddineelamri commented Nov 15, 2022 •
edited by joergsteffens
Loading

Thank you for contributing to the Bareos Project!

Description

show directors command does not work as expected due to a previous update to enums.
This PR fixes the issue.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
  • Is the PR title usable as CHANGELOG entry?
  • Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/s5321-fix-show-directors branch 3 times, most recently from 4ab25ec to 8b10aea Compare November 21, 2022 12:06
@pstorz pstorz requested a review from joergsteffens November 24, 2022 10:59
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/s5321-fix-show-directors branch from 8b10aea to ef1c73b Compare November 25, 2022 18:00
joergsteffens
joergsteffens requested changes Nov 28, 2022
View reviewed changes
Copy link
Member

@joergsteffens joergsteffens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing shows, that the bconsole show command works as expected.
However, there are (now?) discrepancies between the bconsole show command and the --export-config command line argument:

bconsole:

*show DONTEXIST
Resource DONTEXIST not found

*show job=DONTEXIST
job resource DONTEXIST not found.

Okay.

cmdline:

sbin/bareos_dir-python-bareos  -c etc/bareos --xc job DONTEXIST
No ***UNKNOWN*** resource defined

$ sbin/bareos_dir-python-bareos  -c etc/bareos --xc director
<no outputp>

$ sbin/bareos_dir-python-bareos  -c etc/bareos --xc DONTEXIST
<no outputp>

--xc director doesn't work. Missing error output.

Other differences:

  • bconsole:
    • show job => show jobdefs
    • show jobs => show jobs
  • cmd:
    • --xc job => show jobs
    • --xc jobs => <empty>

The job and jobs keywords are handled differently.

core/src/dird/ua_output.cc Outdated
switch (type) {
case -1: /* all */
ShowAll(ua, hide_sensitive_data, verbose);
case -1:
Copy link
Member

@joergsteffens joergsteffens Nov 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you define these magic numbers somewhere?

@joergsteffens
Copy link
Member

joergsteffens commented Nov 28, 2022

Please also run bareos-check-sources and maybe you can find a better usable title to also be addable to the CHANGELOG file.

@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/s5321-fix-show-directors branch from ef1c73b to b3b2750 Compare November 29, 2022 15:23
@alaaeddineelamri
Copy link
Contributor Author

alaaeddineelamri commented Nov 29, 2022

The differences between show and export config were already there on master, so the changes here did not add anything new to that.

The issue with

$ sbin/bareos_dir-python-bareos  -c etc/bareos --xc director
<no outputp>

is the same thing in relation to the enum changes, but not directly involved with the show command. I pushed a fix for that too.

The show command and the export config option do not use exactly the same code. In case we want them to behave the same, I can implement changes for that. Otherwise, if we want to keep this PR uniquely for the purpose of fixing the director config not showing up, we can do the unification of behavior in another PR.

@alaaeddineelamri alaaeddineelamri changed the title dird: fix show directors not working as expected dird: fix director resource not showing when using show director or the --xc director cli option Nov 29, 2022
@joergsteffens joergsteffens force-pushed the dev/alaaeddineelamri/master/s5321-fix-show-directors branch from 2438514 to bb3ce33 Compare November 30, 2022 10:11
@joergsteffens joergsteffens merged commit 3a8de24 into bareos:master Nov 30, 2022
@alaaeddineelamri alaaeddineelamri mentioned this pull request Nov 30, 2022
Merged
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joergsteffens joergsteffens joergsteffens approved these changes

Assignees

@joergsteffens joergsteffens

@alaaeddineelamri alaaeddineelamri

Labels

requires backport to 21

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alaaeddineelamri @joergsteffens