Skip to content
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

DeviceType_Count on Manufacturer #17976

Open
ZionDials opened this issue Nov 11, 2024 · 5 comments
Open

DeviceType_Count on Manufacturer #17976

ZionDials opened this issue Nov 11, 2024 · 5 comments
Assignees
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@ZionDials
Copy link

Deployment Type

Self-hosted

Triage priority

I volunteer to perform this work (if approved)

NetBox Version

v4.1.6

Python Version

3.10

Steps to Reproduce

GET /api/dcim/device-types/

{
    "count": 2424,
    "next": "https://netbox.com/api/dcim/device-types/?limit=50&offset=50",
    "previous": null,
    "results": [
        {
            "id": 1112,
            "url": "https://netbox.com/api/dcim/device-types/1112/",
            "display_url": "https://netbox.com/dcim/device-types/1112/",
            "display": "Thunder 4440 ADC",
            "manufacturer": {
                "id": 47,
                "url": "https://netbox.com/api/dcim/manufacturers/47/",
                "display": "A10",
                "name": "A10",
                "slug": "a10",
                "description": ""
            },
            "default_platform": null,
            "model": "Thunder 4440 ADC",
            "slug": "a10-thunder-4440-adc",
            "part_number": "TH4440",
            "u_height": 1.0,
            "exclude_from_utilization": false,
            "is_full_depth": true,
            "subdevice_role": null,
            "airflow": null,
            "weight": null,
            "weight_unit": null,
            "front_image": null,
            "rear_image": null,
            "description": "",
            "comments": "[A10 Thunder 4440 Applcation Delivery Controller Datasheet](https://www.a10networks.com/wp-content/uploads/A10-DS-Thunder-ADC.pdf)",
            "tags": [],
            "custom_fields": {},
            "created": "2024-10-28T06:49:53.123905-04:00",
            "last_updated": "2024-10-28T06:49:53.123918-04:00",
            "device_count": 0,
            "console_port_template_count": 0,
            "console_server_port_template_count": 0,
            "power_port_template_count": 0,
            "power_outlet_template_count": 0,
            "interface_template_count": 8,
            "front_port_template_count": 0,
            "rear_port_template_count": 0,
            "device_bay_template_count": 0,
            "module_bay_template_count": 0,
            "inventory_item_template_count": 0
        }
    ]
}

Note "devicetype_count" is missing.

Expected Behavior

The expected response, according to the OpenAPI specification for Netbox:

GET /api/dcim/device-types/

{
    "count": 2424,
    "next": "https://netbox.com/api/dcim/device-types/?limit=50&offset=50",
    "previous": null,
    "results": [
        {
            "id": 1112,
            "url": "https://netbox.com/api/dcim/device-types/1112/",
            "display_url": "https://netbox.com/dcim/device-types/1112/",
            "display": "Thunder 4440 ADC",
            "manufacturer": {
                "id": 47,
                "url": "https://netbox.com/api/dcim/manufacturers/47/",
                "display": "A10",
                "name": "A10",
                "slug": "a10",
                "description": "",
                "devicetype_count": 1
            },
            "default_platform": null,
            "model": "Thunder 4440 ADC",
            "slug": "a10-thunder-4440-adc",
            "part_number": "TH4440",
            "u_height": 1.0,
            "exclude_from_utilization": false,
            "is_full_depth": true,
            "subdevice_role": null,
            "airflow": null,
            "weight": null,
            "weight_unit": null,
            "front_image": null,
            "rear_image": null,
            "description": "",
            "comments": "[A10 Thunder 4440 Applcation Delivery Controller Datasheet](https://www.a10networks.com/wp-content/uploads/A10-DS-Thunder-ADC.pdf)",
            "tags": [],
            "custom_fields": {},
            "created": "2024-10-28T06:49:53.123905-04:00",
            "last_updated": "2024-10-28T06:49:53.123918-04:00",
            "device_count": 0,
            "console_port_template_count": 0,
            "console_server_port_template_count": 0,
            "power_port_template_count": 0,
            "power_outlet_template_count": 0,
            "interface_template_count": 8,
            "front_port_template_count": 0,
            "rear_port_template_count": 0,
            "device_bay_template_count": 0,
            "module_bay_template_count": 0,
            "inventory_item_template_count": 0
        }
    ]
}

Observed Behavior

According to the OpenAPI specification BriefManufacturer has a required field of "devicetype_count". However, when querying DeviceTypes and ModuleTypes, the field is not populated and causes SDKs using the OpenAPI generator, to fail.

Go-Netbox Creating Manufacture errors out because of missing required property devicetype_count #165

@ZionDials ZionDials added status: needs triage This issue is awaiting triage by a maintainer type: bug A confirmed report of unexpected behavior in the application labels Nov 11, 2024
@jeremystretch
Copy link
Member

The devicetype_count field is being omitted from the nested representation of manufacturers because the dynamic annotation logic does not recurse for nested serializers. (The field is populated as expected when querying the manufacturers endpoint with ?brief=True.)

While it's feasible to fix this by explicitly attaching the annotation to DeviceTypeViewSet, we should instead extend the dynamic logic to account for nested fields, as it's very likely there are similar instances in other nested serializers that also need to be addressed.

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation severity: medium Results in substantial degraded or broken functionality for specfic workflows and removed status: needs triage This issue is awaiting triage by a maintainer labels Nov 12, 2024
@jeremystretch
Copy link
Member

@ZionDials just realized that you volunteered for this. Assigning to you, thanks!

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Nov 12, 2024
@jeremystretch
Copy link
Member

@ZionDials are you still planning to work on this?

@jeremystretch jeremystretch added severity: low Does not significantly disrupt application functionality, or a workaround is available and removed severity: medium Results in substantial degraded or broken functionality for specfic workflows labels Nov 19, 2024
@ZionDials
Copy link
Author

I've made several efforts to analyze the situation and identify a solution; however, I realize that I don't have the necessary expertise to resolve this issue effectively.

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels Nov 20, 2024
@arthanson arthanson self-assigned this Dec 3, 2024
@arthanson arthanson added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Dec 3, 2024
@arthanson
Copy link
Collaborator

Updated get_annotations_for_serializer which returns the correct annotations, but they aren't displayed.

def get_annotations_for_serializer(serializer_class, fields_to_include=None, annotations=None, base_name=''):
    """
    Return a mapping of field names to annotations to be applied to the queryset for a serializer.
    """
    if annotations is None:
        annotations = {}

    # If specific fields are not specified, default to all
    if not fields_to_include:
        fields_to_include = serializer_class.Meta.fields

    model = serializer_class.Meta.model

    for field_name, field in serializer_class._declared_fields.items():
        if field_name in fields_to_include:
            serializer_field = serializer_class._declared_fields.get(field_name)

            if type(field) is RelatedObjectCountField:
                related_field = model._meta.get_field(field.relation).field
                annotations[base_name + field_name] = count_related(related_field.model, related_field.name)
            elif issubclass(type(serializer_field), Serializer):
                subfields = serializer_field.Meta.brief_fields if serializer_field.nested else None
                annotations = get_annotations_for_serializer(serializer_field, subfields, annotations, f'{field_name}__')

    return annotations

might need to add the annotation directly on the prefetch, along the lines of:

queryset = Post.objects.all()\
    .prefetch_related(
        Prefetch(
            'comments',
             queryset=Comment.objects\
                 .filter(parent__isnull=True)\
                 .annotate(likes=Count('votes__choice'))\
                 .prefetch_related(
                    'children',
                    queryset=Comments.objects.all()\
                    .annotate(likes=Count('votes__choice'))
                 )
        )
    )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

3 participants