-
Notifications
You must be signed in to change notification settings - Fork 0
Added 4 more methods #10
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
base: master
Are you sure you want to change the base?
Changes from all commits
d4f3ffc
e692a16
bf6061a
094ba1a
bb502d8
d84527d
1c7e0a6
d590d81
8dec1b2
271b0e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| from functools import lru_cache | ||
| from resin import Resin | ||
| import json | ||
|
|
||
|
|
||
| class ResinReleaser: | ||
|
|
@@ -21,6 +22,62 @@ def get_canaries(self): | |
| if tag['tag_key'] == 'CANARY'] | ||
| return canaries | ||
|
|
||
| def get_tags_per_device(self): | ||
| all_devices = self.get_all_devices() | ||
| tags = self.models.tag.device.get_all_by_application(self.app_id) | ||
| device_dict = {} | ||
| for device in all_devices.values(): | ||
| device_id = device['id'] | ||
| device_dict[device_id] = { | ||
| 'uuid': device['uuid'], | ||
| 'tags': {}, | ||
| } | ||
| for elem in tags: | ||
| device_id = elem['device']['__id'] | ||
| tag_key = elem['tag_key'] | ||
| value = elem['value'] | ||
| device_dict[device_id]['tags'][tag_key] = value | ||
| tags_per_device = [device_dict[device] for device in list(device_dict.keys())] | ||
| return tags_per_device | ||
|
|
||
| def get_app_env_vars(self): | ||
| all_vars = self.models.environment_variables.application.get_all(int(self.app_id)) | ||
| return [{'id':var['id'], var['name']:var['value']} for var in all_vars] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh the original line got refactored but #10 (comment) still applies |
||
|
|
||
| def get_device_env_vars(self): | ||
| list_of_env_vars_per_device = [] | ||
| tags_per_device = self.get_tags_per_device() | ||
| uuid_list = [device['uuid'] for device in tags_per_device] | ||
| for device in uuid_list: | ||
| allvars = self.models.environment_variables.device.get_all(device) | ||
| list_of_vars = [{'id':var['id'], var['name']:var['value']} for var in allvars] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems very similar to the code in get_app_env_vars, consider putting that in a separate function. |
||
| list_of_env_vars_per_device.append({device:list_of_vars}) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pretty much the comment on structure as above: Any particular reason to choose this... over just this? |
||
| return list_of_env_vars_per_device | ||
|
|
||
| def set_device_env_vars_from_tags(self, tags_per_device, device_env_vars): | ||
| for device in tags_per_device: | ||
| uuid = device['uuid'] | ||
| print("----------------------------------------------------------") | ||
| print("Device: ", uuid) | ||
| try: | ||
| value = json.dumps(device['tags']) | ||
| self.models.environment_variables.device.create(device['uuid'], 'DEVICE_TAG_LIST', value) | ||
| print("creating/overriding var: ", device['uuid'], 'DEVICE_TAG_LIST', value) | ||
| except: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This "except" is too broad and could hide other problems. This is something pylint would point out, I think this project is missing some integration there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted, for sure it will require some more refactoring and integrations in the future but for now I think we can merge it since we need to provide the tags to the ops team. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the pylint stuff is out of scope. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this isn't resolved, the except is still too broad |
||
| print("env var 'DEVICE_TAG_LIST' already exists!") | ||
| for elem in device_env_vars: | ||
| if uuid in elem: | ||
| list_of_env_vars = elem[uuid] | ||
| for var in list_of_env_vars: | ||
| if 'DEVICE_TAG_LIST' in var: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remember to refactor this whole loop after get_app_env_vars returns a plain dict. |
||
| var_id = var['id'] | ||
| old_val = var['DEVICE_TAG_LIST'] | ||
| if old_val != value: | ||
| print("Updating var DEVICE_TAG_LIST with value", value) | ||
| self.models.environment_variables.device.update(var_id, value) | ||
| else: | ||
| print("Not updating var DEVICE_TAG_LIST since the value did not change!") | ||
|
|
||
| @lru_cache() | ||
| def get_releases(self): | ||
| releases = self.models.release.get_all_by_application(self.app_id) | ||
|
|
||
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.
maybe it makes it clearer to have a short docstring here?
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.
maybe