Skip to content

Commit

Permalink
Fix fetching network from location header (#258)
Browse files Browse the repository at this point in the history
* Extract fetching resource by location to separate method

Fixes failing `network create` tests

* Use `field_for_endpoint()`

* Prefer using location header verbatim

Adds handling for endpoints that return invalid location headers.

* Remove `field_for_endpoint()`

* Remove placeholder comment, move src comments

* Remove special label handling, never re-fetch labels
  • Loading branch information
pederhan authored Jun 7, 2024
1 parent 205da62 commit 86a5f25
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 46 deletions.
23 changes: 2 additions & 21 deletions mreg_cli/api/abstracts.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,6 @@ def id_for_endpoint(self) -> int | str:
field = self.endpoint().external_id_field()
return getattr(self, field)

@classmethod
def field_for_endpoint(cls) -> str:
"""Return the appropriate field for the object for its endpoint.
:param field: The field to return.
:returns: The correct field for the endpoint.
"""
return cls.endpoint().external_id_field()

@classmethod
@abstractmethod
def endpoint(cls) -> Endpoint:
Expand Down Expand Up @@ -408,7 +399,7 @@ def refetch(self) -> Self:
:returns: The fetched object.
"""
id_field = self.field_for_endpoint()
id_field = self.endpoint().external_id_field()
identifier = getattr(self, id_field)

if not identifier:
Expand Down Expand Up @@ -486,17 +477,7 @@ def create(cls, params: JsonMapping, fetch_after_create: bool = True) -> Self |
if response and response.ok:
location = response.headers.get("Location")
if location and fetch_after_create:
obj = None
if cls.endpoint().external_id_field() == "name":
obj = cls.get_by_field("name", location.split("/")[-1])
else:
obj = cls.get_by_id(int(location.split("/")[-1]))

if obj:
return obj

raise GetError(f"Could not fetch object from location {location}.")

return get_typed(location, cls)
# else:
# Lots of endpoints don't give locations on creation,
# so we can't fetch the object, but it's not an error...
Expand Down
27 changes: 2 additions & 25 deletions mreg_cli/api/endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,10 @@
from __future__ import annotations

from enum import Enum
from typing import Any, Callable
from mreg_cli.outputmanager import OutputManager
from typing import Literal
from urllib.parse import quote, urljoin


class hybridmethod:
"""Decorator to allow a method to be called both as a class method and an instance method."""

def __init__(self, func: Callable[..., Any]):
"""Initialize the hybrid method."""
self.func = func

def __get__(self, obj: object | None, cls: type | None = None):
"""Return a method that can be called both as a class method and an instance method."""
if obj is None:
return classmethod(self.func).__get__(None, cls)
else:
# Called on an instance, act like an instance method
return self.func.__get__(obj)

def __call__(self, *args: Any, **kwargs: Any):
"""Caller method."""
return self.func(*args, **kwargs)


class Endpoint(str, Enum):
"""API endpoints."""

Expand Down Expand Up @@ -107,8 +86,7 @@ def requires_search_for_id(self) -> bool:
"""Return True if this endpoint requires a search for an ID."""
return self.external_id_field() != "id"

@hybridmethod
def external_id_field(self) -> str:
def external_id_field(self) -> Literal["id", "name", "network", "host"]:
"""Return the name of the field that holds the external ID."""
if self in (
Endpoint.Hosts,
Expand All @@ -128,7 +106,6 @@ def external_id_field(self) -> str:
return "host"
return "id"

# Add methods via composition
def with_id(self, identity: str | int) -> str:
"""Return the endpoint with an ID."""
id_field = quote(str(identity))
Expand Down

0 comments on commit 86a5f25

Please sign in to comment.