-
-
Notifications
You must be signed in to change notification settings - Fork 298
[feature] Mass command asynchronous execution pipeline #1395
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: gsoc26-mass-commands
Are you sure you want to change the base?
Changes from all commits
ffb86de
76982cd
82f104c
b0ef6f4
0d7aa65
2ff7258
65f73d0
e9bf89f
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 |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| DeviceConnection = load_model("connection", "DeviceConnection") | ||
| Credentials = load_model("connection", "Credentials") | ||
| Device = load_model("config", "Device") | ||
| BatchCommand = load_model("connection", "BatchCommand") | ||
|
|
||
|
|
||
| class ValidatedDeviceFieldSerializer(ValidatedModelSerializer): | ||
|
|
@@ -43,6 +44,10 @@ class CommandSerializer(ValidatedDeviceFieldSerializer): | |
| required=False, | ||
| pk_field=serializers.UUIDField(format="hex_verbose"), | ||
| ) | ||
| batch_command = serializers.PrimaryKeyRelatedField( | ||
| read_only=True, | ||
| pk_field=serializers.UUIDField(format="hex_verbose"), | ||
| ) | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
|
|
@@ -115,3 +120,102 @@ class Meta: | |
| "is_working": {"read_only": True}, | ||
| } | ||
| read_only_fields = ("created", "modified") | ||
|
|
||
|
|
||
| class BatchCommandExecuteSerializer( | ||
| FilterSerializerByOrgManaged, serializers.ModelSerializer | ||
| ): | ||
| type = serializers.CharField() | ||
| input = serializers.JSONField(allow_null=True, required=False) | ||
| devices = serializers.PrimaryKeyRelatedField( | ||
| many=True, | ||
| queryset=Device.objects.all(), | ||
| required=False, | ||
| allow_empty=True, | ||
| pk_field=serializers.UUIDField(format="hex_verbose"), | ||
| ) | ||
| execute_all = serializers.BooleanField(required=False, default=True) | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| super().__init__(*args, **kwargs) | ||
| request = self.context.get("request") | ||
| if request and request.method == "GET": | ||
| self.fields["type"].required = False | ||
|
|
||
| class Meta: | ||
| model = BatchCommand | ||
| fields = ( | ||
| "organization", | ||
| "type", | ||
| "input", | ||
| "devices", | ||
| "group", | ||
| "location", | ||
| "execute_all", | ||
| ) | ||
| extra_kwargs = { | ||
| "organization": {"required": False, "allow_null": True}, | ||
| } | ||
|
|
||
| def validate(self, data): | ||
| org = data.get("organization") | ||
| execute_all = data.get("execute_all", False) | ||
| devices = data.get("devices") | ||
| group = data.get("group") | ||
| location = data.get("location") | ||
| if not org and not self.context["request"].user.is_superuser: | ||
| raise serializers.ValidationError( | ||
| _("Only superusers can execute batch commands without an organization.") | ||
| ) | ||
|
Comment on lines
+166
to
+169
Member
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. Can you check if we have existing mixins in openwisp-users which can perform this operation?
Member
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. No existing DRF serializer-level mixin in openwisp-users performs this check. |
||
| if not execute_all and not org and not devices and not group and not location: | ||
| raise serializers.ValidationError( | ||
| _( | ||
| "Specify at least one targeting option " | ||
| "or set execute_all to true." | ||
| ) | ||
| ) | ||
| if devices: | ||
| for device in devices: | ||
| if org and device.organization_id != org.id: | ||
| raise serializers.ValidationError( | ||
| { | ||
| "devices": _( | ||
| "All devices must belong to the same organization." | ||
| ) | ||
| } | ||
| ) | ||
| return data | ||
|
Comment on lines
+177
to
+187
Member
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 see the Model also performs this validation. And, it does so more elegantly. |
||
|
|
||
|
|
||
| class BatchCommandSerializer(BaseSerializer): | ||
| device_count = serializers.IntegerField(source="devices.count", read_only=True) | ||
|
|
||
| class Meta: | ||
| model = BatchCommand | ||
| fields = ( | ||
| "id", | ||
| "organization", | ||
| "status", | ||
| "type", | ||
| "input", | ||
| "group", | ||
| "location", | ||
| "device_count", | ||
| "created", | ||
| "modified", | ||
| ) | ||
| read_only_fields = ( | ||
| "created", | ||
| "modified", | ||
| ) | ||
|
|
||
|
|
||
| class BatchCommandDetailSerializer(BatchCommandSerializer): | ||
| devices = serializers.PrimaryKeyRelatedField( | ||
| many=True, | ||
| read_only=True, | ||
| pk_field=serializers.UUIDField(format="hex_verbose"), | ||
| ) | ||
|
|
||
| class Meta(BatchCommandSerializer.Meta): | ||
| fields = BatchCommandSerializer.Meta.fields + ("devices",) | ||
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.
Why do we need this?
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.
For POST requests, the type field is mandatory. However, for GET requests we only need the dry-run functionality to retrieve the list of devices, so requiring type for every GET request does not make much sense.
At the moment, the endpoint requires requests like:
http://0.0.0.0:8000/api/v1/controller/batch-command/execute/?type=reboot
even for GET requests where the type value is not actually used.
Creating a separate serializer just for the GET request felt like unnecessary complexity, so I decided to handle it this way for now.
Let me know if there is a cleaner or more appropriate way to handle this.