Openskin white list systems
As discussed in the December meeting, we might want to white list systems that are compatible with the current version of openskin .
Comments (48)
-
reporter -
reporter checks overule option refs
#888→ <<cset b47564b78f79>>
-
reporter Hi@Luuk , I’ve added an “ignore whitelist” option in Django forms, a fixture dir, fixture dir in settings. It now checks whether the whitelist must be ignored and if not, it checks for each skin dose calculation if the system is on the whitelist. If the software version field is left blank, all software versions are allowed.
Is this close to what you had in mind?
Left to do:
- in case we do not want to ignore the whitelist and a system is not found suitable, what can we do to prevent the calculation restarting upon refreshing the page. (e.g. on our server we serialise “empty” structs i.e. skin_map = {0,0})
- something about django tests because the pipeline builds fail at the moment
- …?
-
Just a note: this is the same issue as
#883 -
reporter @Luuk Whoops, I don't know why I could not find it before.
Latest updates:
- I implemented the software version check as you suggested, i.e. equal and newer software versions are allowed.
- Mentioned the overrule option shortly in the docs (I think we should elaborate it perhaps?), I think I also want to mention that an advanced user can change this fixture file to add systems that the user trusts (otherwise whitelisting is very very binary and limited to the systems we provided), but I do not know how this works with the docker installed version. For now I’ve added the fixture dir to the settings file, manage.py loaddata openskin_white_list.json does the trick.
Whenever make_skin_map is called, it checks the system with the whitelisted systems, if no software version is specified in the fixture, all software version are allowed. Otherwise the versions are compared. I’ve separated the part in make_skin_map.py in a separate module to make it a bit more versatile.
As for the pipeline test, it fails. I’m not so sure why, everything works fine om my computer but perhaps this could be due to the fixture not being loaded during the tests. @Ed McDonagh what do you think?
-
Issue
#883was marked as a duplicate of this issue. -
The first test that fails @wens is test_skin_map_zee. This just treats OpenSkin as a black box -
test_files/RF-RDSR-Siemens-Zee.dcm
is imported, callstools.make_skin_map.make_skin_map
on the import, then finds the saved pickle file by reconstructing the path and checks the content.In this branch, the file is found, but the content is
{'skin_map': [0, 0], 'skin_map_version': '0.8'}.
This is why the test fails. What do you expect the content to be? -
reporter @Ed McDonagh @Luuk Hi Ed and Luuk, that result is exactly what I would expect with no fixture files loaded, I’ve added a lines to the test file that loads the fixtures but I was wondering whether it would be more logical to do that in the pipeline yml definition.
-
Loading fixtures using class attribute of TestCase. Also loading settings from django.conf. Refs
#888→ <<cset 5efcaa12cc83>>
-
Loading fixtures in the same way in the other test. All tests now passing locally. Refs
#888→ <<cset d0db3f3c1f90>>
-
Existing tests now fixed @wens
-
Not tested yet, but I think this is what is needed for the docs. Refs
#888[skip ci] docs only→ <<cset 9ab518597752>>
-
reporter codacy issues refs
#888→ <<cset 8050aa0e5c01>>
-
Initial implementation of adding skin map enabled yes or no column to display names view. Not properly tested, will link to view to enable/disable. Refs
#888→ <<cset 220464d7fabc>>
-
Incidental html corrections. Refs
#888changes.→ <<cset c074292aee8e>>
-
Missing quote. Refs
#888changes→ <<cset 4af278fac848>>
-
Removing check of version against packaging version parse, fluoro versions rarely use Python style versioning. Refs
#888→ <<cset 0dd414cbfd9c>>
-
Switching between model, and model + version now works. Need to add add, delete, docs, and edge cases. Refs
#888→ <<cset a1989ea3c163>>
-
Added adding. Not tested model add, as run out of systems! Need to add delete next. Refs
#888→ <<cset 85882934479a>>
-
Initial delete. Revamped display_skin_enabled. Revamped add template. Need to redo modify and delete. Refs
#888→ <<cset 6fa165e090f5>>
-
Code reformatting and minor display modification. Refs
#888→ <<cset 83a624eb10e9>>
-
Basically works, but ugly. Refs
#888→ <<cset 237cba118479>>
-
Fixed the logic, now works in all directions. Refs
#888→ <<cset 310bbbc1b809>>
-
Improving variable and function names. Refs
#888→ <<cset 8ea959b84564>>
-
Moved openskin related views to new file, marginally reducing size of views_admin.py. Refs
#888→ <<cset e6e1ea7bdb25>>
-
Using PEP 3135 -- New Super to satisfy Codacy. Refs
#888→ <<cset 8e2c1bf2eec9>>
-
Attempt to satisfy Codacy return policy. Refs
#888→ <<cset f6dcc9958154>>
-
Refactoring to reduce duplication. Refs
#888→ <<cset 91b5d557a29e>>
-
Made the formatting of the 'How many studies' and 'Date of last study' fields on the display name page match the other fields. Refs issue
#888→ <<cset be54b917baeb>>
-
New allow_safelist_modify bool added, changes whether link to modify/add/delete appears so far. Refs
#888→ <<cset f67665e1f009>>
-
Check if admin before adding new item to list. Refs
#888→ <<cset bd18430866fd>>
-
Codacy nitpicking. Refs
#888→ <<cset ef6b0c0bf357>>
-
Adding docstrings - looking at switching to numpy style. Refs
#888→ <<cset 70fb4971cbdd>>
-
Trying to fix import errors in docs generation. [skip ci] docs only. Refs
#888→ <<cset 689a30d2017b>>
-
Adding docstring for get_matching_equipment_names. [skip ci] docs only. Refs
#888→ <<cset 117a6f99a1ff>>
-
Added more docstrings. Need to check 'allow modify' is respected, links between settings and display names, remove ignore safelist option. [skip ci] docs only. Refs
#888→ <<cset 6dc99a26eed9>>
-
Attempts to satisfy Codacy. [skip ci] docs only. Refs
#888→ <<cset 987128d07167>>
-
Endsure list cannot be added to if update not allowed. Refs
#888→ <<cset 93e52f32d6e5>>
-
Adding check on allow safelist modify to safelist modify! Refs
#888→ <<cset f29b1669f3c6>>
-
Overriding the delete function to check allow_safelist_modify. Refs
#888→ <<cset 1c1a25973e12>>
-
Removing overrule_safelist option, in favour of allow_safelist_modify. Refs
#888→ <<cset a88c92924b32>>
-
Updating docs link to the skin dose map page - doesn't currently have content for managing the safelist. Refs
#888→ <<cset fb05ce903bc9>>
-
Forgot to run black. Refs
#888→ <<cset d4799dfd8371>>
-
Adding in some tests to ensure changing the safe list actually makes a difference! Refs
#888→ <<cset 645772a0b18f>>
-
Merged in issue888updatelist (pull request #453)
Refs
#888- adds facility to review and update the safe list via the display name config page.→ <<cset 627d7f8d81bb>>
-
Refactored imports, remoted try except which didn't look necessary. Refs
#888→ <<cset 21fd75d57408>>
-
- changed status to resolved
Merged in issue888OpenSkinWhiteList (pull request #445)
Fixes
#888OpenSkinWhiteListDocumentation needs to follow. Approved-by: Ed McDonagh
→ <<cset c2140857fde1>>
-
@wens @Luuk @David Platten @Jonathan Cole this has now been deployed to dev.openrem.org if you have an admin login?
One thing I notice is that if a dose map exists, it is still displayed when the system is not on the safe list. I don’t know if that is desirable or not…
- Log in to comment
(crude) approach to system white list based on manufacturer, model name and software version, added a check before calculating the skin map, white listed systems should be added to the fixture file. Refs issue
#888→ <<cset 977789d4b2e2>>