-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create and use constants for landing zone status types #1729
Create and use constants for landing zone status types #1729
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! I can see two issues:
Issue 1
Importing large amounts of these constants in modules can bloat up the import sections quite a bit. I suggest you move these and the other landingzones constants into landingzones/constants.py
. Then, if you need a big bunch of these constants imported in a single module, you can instead do:
import landingzones.constants as lc
do_something_with(lc.ZONE_STATUS_OK) # Etc etc.
Issue 2
Grep shows me there are still places where we refer to these status types by string instead of the constant. Mostly in tests but also elsewhere. Example:
(sodar14) mikkopen@cubi-7300-mn:~/code/sodar$ grep -niIr --include "*.py" "'MOVED'"
samplesheets/views.py:1358: status__in=['MOVED', 'DELETED']
landingzones/management/commands/inactivezones.py:23: ).exclude(status__in=('DELETED', 'MOVED'))
landingzones/plugins.py:128: if obj.__class__ == LandingZone and obj.status != 'MOVED':
landingzones/plugins.py:169: active_count = zones.exclude(status__in=['MOVED', 'DELETED']).count()
landingzones/models.py:24:ZONE_STATUS_MOVED = 'MOVED'
landingzones/models.py:75:STATUS_FINISHED = ['MOVED', 'NOT CREATED', 'DELETED']
irodsadmin/management/commands/irodsorphans.py:87: ~(Q(status='MOVED') & Q(status='DELETED'))
taskflowbackend/tests/test_flows.py:632: self.assertEqual(self.zone.status, 'MOVED')
taskflowbackend/tests/test_flows.py:691: self.assertEqual(self.zone.status, 'MOVED')
taskflowbackend/tests/test_flows.py:763: self.assertEqual(self.zone.status, 'MOVED')
taskflowbackend/tests/test_flows.py:967: self.assertEqual(new_zone.status, 'MOVED')
taskflowbackend/flows/landing_zone_move.py:329: 'status': 'MOVED',
We should consistently use the constants everywhere.
d670085
to
ea0a627
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No notes, good work! Will be merged :)
Issue #1398
TODO: