From e5e24d6b724c60d6c13987252941ae5bb6bc88ef Mon Sep 17 00:00:00 2001
From: Arthur Degonde <44548105+ArthurD1@users.noreply.github.com>
Date: Thu, 14 Nov 2024 11:26:58 +0100
Subject: [PATCH] fix: httpclient doc (#553)

---
 docs/apps/http_client/examples/full.yml                |  8 ++++----
 docs/apps/http_client/examples/simple.yml              |  2 +-
 harp/config/configurables/service.py                   |  6 +++++-
 harp/config/tests/__snapshots__/test_all_examples.ambr | 10 ++++------
 harp/config/tests/test_all_examples.py                 |  1 +
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/docs/apps/http_client/examples/full.yml b/docs/apps/http_client/examples/full.yml
index 8fddc9c7..fa5852d7 100644
--- a/docs/apps/http_client/examples/full.yml
+++ b/docs/apps/http_client/examples/full.yml
@@ -5,7 +5,7 @@ http_client:
   # Customize the httpx transport layer (optional)
   transport:
     # This is the default, only set if you want to use a custom transport. Most users don't need to set this.
-    "@type": "httpx:AsyncHTTPTransport"
+    type: httpx.AsyncHTTPTransport
 
   # Cache configuration (optional, enabled by default)
   cache:
@@ -15,7 +15,7 @@ http_client:
     # Override the cache controller definition (optional)
     controller:
       # This is the default, only set if you want to use a custom controller.
-      "@type": "hishel:Controller"
+      type: hishel.Controller
 
       # You can configure anything the hishel cache controller would accept as a keyword argument.
       # See https://hishel.com/advanced/controllers/ for more information.
@@ -36,7 +36,7 @@ http_client:
     # layer extending the base http client features with caching.
     transport:
       # This is the default, only set if you want to use a custom cache transport.
-      "@type": "hishel:AsyncCacheTransport"
+      type: hishel.AsyncCacheTransport
 
       # If your hishel compatible transport class take more kwargs, you can pass more stuff here.
       # See https://hishel.com/userguide/#clients-and-transports
@@ -45,7 +45,7 @@ http_client:
     # Please note that we plan to allow to share other harp storages here in the future.
     storage:
       # This is the default, only set if you want to use a custom cache storage.
-      "@type": "hishel:AsyncFileStorage"
+      type: hishel.AsyncFileStorage
 
       # If your hishel compatible storage class take more kwargs, you can pass more stuff here.
       # See https://hishel.com/advanced/storages/
diff --git a/docs/apps/http_client/examples/simple.yml b/docs/apps/http_client/examples/simple.yml
index 61d848ea..0af5a3a5 100644
--- a/docs/apps/http_client/examples/simple.yml
+++ b/docs/apps/http_client/examples/simple.yml
@@ -7,7 +7,7 @@ http_client:
   transport:
     # You can customize the default transport implementation. The values shown are the
     # defaults, and you only need to set them if you want to provide different values.
-    "@type": "httpx:AsyncHTTPTransport"
+    type: httpx.AsyncHTTPTransport
     retries: 0
     verify: true
     http1: true
diff --git a/harp/config/configurables/service.py b/harp/config/configurables/service.py
index fdc86a2f..0658af70 100644
--- a/harp/config/configurables/service.py
+++ b/harp/config/configurables/service.py
@@ -1,3 +1,4 @@
+from keyword import iskeyword
 from typing import Optional
 
 from pydantic import ConfigDict, Field, model_serializer, model_validator
@@ -30,7 +31,10 @@ def __validate(cls, values):
         arguments = values.get("arguments", {})
         for k in list(values.keys()):
             if k not in cls.model_fields:
-                arguments[k] = values.pop(k)
+                if str.isidentifier(k) and not iskeyword(k):
+                    arguments[k] = values.pop(k)
+                else:
+                    raise ValueError(f"Invalid field name: {k} for {cls.__name__}")
         if len(arguments):
             return {**values, "arguments": arguments}
         return values
diff --git a/harp/config/tests/__snapshots__/test_all_examples.ambr b/harp/config/tests/__snapshots__/test_all_examples.ambr
index 5eb66d56..64d7581a 100644
--- a/harp/config/tests/__snapshots__/test_all_examples.ambr
+++ b/harp/config/tests/__snapshots__/test_all_examples.ambr
@@ -132,7 +132,6 @@
   http_client:
     cache:
       controller:
-        '@type': hishel:Controller
         allow_heuristics: true
         allow_stale: true
         cacheable_methods:
@@ -142,13 +141,12 @@
         - 200
         - 301
         - 308
+        type: hishel.Controller
       storage:
-        '@type': hishel:AsyncFileStorage
-      transport:
-        '@type': hishel:AsyncCacheTransport
+        type: hishel.AsyncFileStorage
     timeout: 10.0
     transport:
-      '@type': httpx:AsyncHTTPTransport
+      type: httpx.AsyncHTTPTransport
   proxy: {}
   storage: {}
   
@@ -171,10 +169,10 @@
   http_client:
     timeout: 10.0
     transport:
-      '@type': httpx:AsyncHTTPTransport
       http1: true
       http2: false
       retries: 0
+      type: httpx.AsyncHTTPTransport
       verify: true
   proxy: {}
   storage: {}
diff --git a/harp/config/tests/test_all_examples.py b/harp/config/tests/test_all_examples.py
index 1350652a..fa542ad9 100644
--- a/harp/config/tests/test_all_examples.py
+++ b/harp/config/tests/test_all_examples.py
@@ -49,6 +49,7 @@ def test_load_documentation_example(configfile, snapshot):
     from harp.config import ConfigurationBuilder
 
     builder = ConfigurationBuilder()
+    ## TODO : remove when rules enabled by default
     if configfile.startswith("docs/apps/rules/"):
         builder.applications.add("rules")
     builder.add_file(os.path.join(harp.ROOT_DIR, configfile))