Bugfix 67214 minion swarm#67783
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey. There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
| import salt.utils.path | ||
| import salt.utils.platform | ||
| import tests.support.paths as paths | ||
| import support.paths as paths |
There was a problem hiding this comment.
The original threw an unfound error. runtests.py exists within salt/tests/support. Relative to runtests.py, paths is not located at tests.support.paths, but rather at support.paths. This may have resulted from a reorganization of the directory structure sometime after the 2015 release in which minionswarm.py and runtests.py were moved (that's a guess).
There was a problem hiding this comment.
I just spent some time reading about relative pathnames in the import statement. I used "import support.paths" simply because it worked. However, after reading, I'm not sure why it works. Perhaps "import paths" would be better, since paths.py exists in the same directory as runtests.py.
Added later: OK, I guess I am getting above my head on this. I just tried changing "import support.paths as paths" to "import paths" and minionswarm.py threw an unfound error. So, someone who has more experience with import statements will have to advise on the best way to import paths.py when it is in the same directory as runtests.py
|
Looks like there are some pre-commit failures. |
OK. I will take a look at it. It may take some time since I am reorganizing my development environment. |
|
I am very new to salt development, so I very possibly am missing something, I ran nox -e lint-tests in bugfix-67214, which is what is pushed to https://github.com/dnessett/salt and what is the basis of the PR. The results are: To check I was in the correct branch, I ran: and: I then checked the changelog directory in case the changelog file was faulty, but it seems not: So, I am stuck finding out what is causing pre-commit to fail. |
|
In general, you should consider installing the |
Thanks. |
Sorry for the delay, I had some emergencies to resolve in another area of my volunteer work. When I run: There doesn't seem to be any errors from black or isort. Perhaps this is because the two files in question have been committed and so black/isort doesn't regard them as changed? (This is a guess, since I don't know what black/iisort checks for) |
|
@dnessett $PWD being |
I'm in the git source directory when I execute the black/isort functions. I am reorganizing my development environment, so I cloned a copy of my github repository to a directory in my home directory in order to address this problem while the reorganization is in process. Just to demonstrate the files are in the right place: |
| default="minion", | ||
| help="Give the minions an alternative id prefix, this is used " | ||
| "when minions from many systems are being aggregated onto " | ||
| "a single master. (default = minion)", |
There was a problem hiding this comment.
Don't duplicate a default value in the help text of an OptionParser argument (the same applies to ArgumentParser). Use %default in the help text. With ArgumentParser you can show the defaults for all arguments by setting formatter_class=argparse.ArgumentDefaultsHelpFormatter. I am not sure if OptionParser has similar functionality or not since I haven't really used because ArgumentParser superseded it over a decade ago.
…Added username to salt-master command. Added ability to turn off open_mode.
clarificationn to the config-dir help text.
…nswarm.py and tests/support/runtests.py
Address review feedback on saltstack#67214: * tests/support/runtests.py: restore canonical package-qualified import. ``import support.paths as paths`` only resolved when the CWD was ``tests/`` and broke ``from tests.support.runtests import RUNTIME_VARS`` -- used by 50+ conftests/tests. Use ``from tests.support import paths`` instead. This was the root cause of the 393 failing Test Salt / Test Package jobs. * tests/minionswarm.py: use ``from tests.support import runtests`` and add a sys.path bootstrap so the script can still be executed directly via ``python tests/minionswarm.py`` (the original ``cd tests/`` reproduction from saltstack#67214 also still works). * tests/minionswarm.py: collapse duplicated ``machine_id`` assignment, fix invalid ``sys.exit([1])`` -> ``sys.exit(1)``. * tests/minionswarm.py: master ``sock_dir`` pointed at ``var/run/salt/minion`` instead of ``var/run/salt/master``. * tests/minionswarm.py: ``--open-mode`` was declared with ``default=True`` and no action, so ``--open-mode=False`` parsed to the string ``"False"`` (truthy). Switched to ``--no-open-mode`` (action="store_false") so the flag does what the help text claims. * tests/minionswarm.py: typo fixes (``/tmp/srooot`` -> ``/tmp/sroot``, ``manaaged`` -> ``managed``, ``alt-master`` -> ``salt-master``).
6 participants
What does this PR do?
Fixes bugs in tests/minionswarm.py that were preventing it to run.
What issues does this PR fix or reference?
Fixes #67214
Previous Behavior
minionswarm failed to run
New Behavior
minionswarm now runs correctly
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
No