[REFACTOR] headers refactor by radoslawrolka · Pull Request #42 · radoslawrolka/ElixirDatasets

radoslawrolka · 2026-01-06T19:09:25Z

📝 Description

headers refactor

🎯 Type of Changes

🔄 Refactoring

Copilot

Pull request overview

This PR refactors header building logic across the codebase by introducing centralized get_headers/3 and get_headers!/3 functions in ElixirDatasets.Utils.HTTP. The refactoring consolidates duplicate header construction code and standardizes how authorization tokens and content-type headers are built throughout the application.

Key changes:

Added new get_headers/3 and get_headers!/3 functions to centralize header building logic with support for auth tokens, custom content types, and accept headers
Replaced inline header building code across multiple modules with calls to the new centralized functions
Updated test fixtures to use charlists for headers instead of strings to match runtime behavior

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
lib/elixir_datasets/utils/http.ex	Adds new `get_headers/3` and `get_headers!/3` functions with helper functions for building headers; removes old `build_headers` function
lib/huggingface/hub.ex	Replaces inline auth header construction with call to `get_headers(opts)`
lib/elixir_datasets.ex	Replaces inline auth header construction with call to `get_headers(opts)`
lib/elixir_datasets/utils/uploader.ex	Replaces multiple instances of inline header construction with calls to `get_headers!`; removes explicit token retrieval and parameter passing in several functions
test/huggingface/hub_test.exs	Updates test headers from strings to charlists to match runtime format
test/elixir_datasets/utils/http_test.exs	Updates test headers from strings to charlists to match runtime format

Comments suppressed due to low confidence (1)

lib/elixir_datasets/utils/uploader.ex:31

The options parameter is not being forwarded to commit_to_huggingface. If a user passes an :auth_token in the options to upload_dataset, it will be ignored because commit_to_huggingface calls get_headers!([]) with an empty options list instead of receiving and using the provided options.

           {:ok, response} <-
             commit_to_huggingface(
               repository,
               filename,
               encoded_content,
               commit_msg,
               description
             ) do

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-06T19:14:34Z

-      {~c"Authorization", ~c"Bearer #{token}"},
-      {~c"Content-Type", ~c"application/x-ndjson"}
-    ]
+    headers = ElixirDatasets.Utils.HTTP.get_headers!([], "application/x-ndjson")


The commit_to_huggingface function should accept an options parameter and pass it to get_headers! to ensure that any auth token provided by the caller is used. Currently, it always calls get_headers!([]) which will only work with environment variables.

Copilot · 2026-01-06T19:14:35Z

-      {~c"Authorization", ~c"Bearer #{token}"},
-      {~c"Content-Type", ~c"application/x-ndjson"}
-    ]
+    headers = ElixirDatasets.Utils.HTTP.get_headers!([], "application/x-ndjson")


The delete_from_huggingface function should accept an options parameter and pass it to get_headers! to ensure that any auth token provided by the caller is used. Currently, it always calls get_headers!([]) which will only work with environment variables.

Copilot · 2026-01-06T19:14:35Z

+    headers = [
+      {~c"user-agent", ~c"elixirDatasets"},
+      {~c"Content-Type", String.to_charlist(content_type)}
+    ]
+
+    add_accept_header(headers, accept)
+    add_auth_header(headers, ElixirDatasets.HuggingFace.Hub.get_auth_token(opts))


The return values of add_accept_header and add_auth_header are not being used. The function returns the original headers list instead of the modified headers. You need to pipe the results or use intermediate variables to chain these operations properly.

Suggested change

headers = [

{~c"user-agent", ~c"elixirDatasets"},

{~c"Content-Type", String.to_charlist(content_type)}

]

add_accept_header(headers, accept)

add_auth_header(headers, ElixirDatasets.HuggingFace.Hub.get_auth_token(opts))

headers =

[

{~c"user-agent", ~c"elixirDatasets"},

{~c"Content-Type", String.to_charlist(content_type)}

]

|> add_accept_header(accept)

|> add_auth_header(ElixirDatasets.HuggingFace.Hub.get_auth_token(opts))

headers

Copilot · 2026-01-06T19:14:35Z

+  @doc """
+  Builds authorization and JSON content-type headers.
+
+  ## Parameters
+
+    * `opts` - keyword list of options, may contain `:auth_token`
+    * `content_type` - the content type to set in the headers, defaults to `application/json`
+
+  ## Returns
+
+    * `headers`
+  """


The documentation is missing the accept parameter. The function signature includes accept as the third parameter, but it's not documented in the "Parameters" section.

Copilot · 2026-01-06T19:14:36Z

+  @doc """
+  Builds authorization and JSON content-type headers.
+
+  Same as `get_headers/2` but raises on error if auth token retrieval fails.


The documentation states "Same as get_headers/2" but the function being documented is get_headers!/3 which takes 3 parameters (with defaults), not 2. This should reference get_headers/3 for accuracy.

Suggested change

Same as `get_headers/2` but raises on error if auth token retrieval fails.

Same as `get_headers/3` but raises on error if auth token retrieval fails.

Copilot · 2026-01-06T19:14:36Z

-       ~c"Bearer #{ElixirDatasets.HuggingFace.Hub.get_auth_token([]) |> elem(1)}"},
-      {~c"Content-Type", ~c"application/json"}
-    ]
+    headers = ElixirDatasets.Utils.HTTP.get_headers!()


The function receives _verify_token as a parameter but doesn't use it. Instead, it calls get_headers!() which will use the HuggingFace auth token. If the verify endpoint requires the verify token from the LFS batch response rather than the auth token, this would be incorrect. Please verify which token should be used for the verify endpoint.

Copilot · 2026-01-06T19:14:36Z

+    add_accept_header(headers, accept)
+    add_auth_header!(headers, ElixirDatasets.HuggingFace.Hub.get_auth_token(opts))


The return values of add_accept_header and add_auth_header! are not being used. The function returns the original headers list instead of the modified headers. You need to pipe the results or use intermediate variables to chain these operations properly.

Copilot · 2026-01-06T19:14:36Z

+  @doc """
+  Builds authorization and JSON content-type headers.
+
+  Same as `get_headers/2` but raises on error if auth token retrieval fails.
+
+  ## Parameters
+
+    * `opts` - keyword list of options, may contain `:auth_token`
+    * `content_type` - the content type to set in the headers, defaults to `application/json`
+
+  ## Returns
+
+    * `headers` - a list of headers including authorization if available
+
+  ## Raises
+
+    * `RuntimeError` - if auth token retrieval fails: "Failed to get Hugging Face auth token when building headers, reason: <reason>"
+  """


The documentation is missing the accept parameter. The function signature includes accept as the third parameter, but it's not documented in the "Parameters" section.

Copilot · 2026-01-06T19:14:36Z

+    headers =
+      ElixirDatasets.Utils.HTTP.get_headers!(
+        [],
+        "application/vnd.git-lfs+json",
+        "application/vnd.git-lfs+json"
+      )


The get_headers! call is passed an empty options list [], which means it won't include the authentication token. The auth token was retrieved earlier in upload_file_via_lfs (line 112), but it's not being passed to initiate_lfs_batch. You should pass the options or token through to this function so the authentication header is included.

Copilot · 2026-01-06T19:14:37Z

-         {:ok, response} <-
-           delete_from_huggingface(repository, token, filename, commit_msg, description) do
+    with {:ok, response} <-
+           delete_from_huggingface(repository, filename, commit_msg, description) do


The options parameter from delete_file_from_dataset is not being forwarded to delete_from_huggingface. If a user passes an :auth_token in the options, it will be ignored because delete_from_huggingface calls get_headers!([]) with an empty options list instead of receiving and using the provided options.

Suggested change

delete_from_huggingface(repository, filename, commit_msg, description) do

delete_from_huggingface(repository, filename, commit_msg, description, options) do

[REFACTOR] headers refactor

0472846

radoslawrolka requested a review from Copilot January 6, 2026 19:09

Copilot started reviewing on behalf of radoslawrolka January 6, 2026 19:09 View session

Copilot AI reviewed Jan 6, 2026

View reviewed changes

radoslawrolka marked this pull request as draft January 11, 2026 10:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] headers refactor#42

[REFACTOR] headers refactor#42
radoslawrolka wants to merge 1 commit into
masterfrom
rolkrado/feature_3

radoslawrolka commented Jan 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Copilot AI Jan 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Same as `get_headers/2` but raises on error if auth token retrieval fails.
	Same as `get_headers/3` but raises on error if auth token retrieval fails.

		add_accept_header(headers, accept)
		add_auth_header!(headers, ElixirDatasets.HuggingFace.Hub.get_auth_token(opts))

	delete_from_huggingface(repository, filename, commit_msg, description) do
	delete_from_huggingface(repository, filename, commit_msg, description, options) do

Conversation

radoslawrolka commented Jan 6, 2026

📝 Description

🎯 Type of Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants