dhall-rust icon indicating copy to clipboard operation
dhall-rust copied to clipboard

Support custom import headers

Open Nadrieril opened this issue 4 years ago • 6 comments

There has been discussion of removing this feature from the language, so I'm not in a hurry to implement it.

Nadrieril avatar Mar 04 '20 21:03 Nadrieril

There was discussion indeed https://github.com/dhall-lang/dhall-lang/issues/543, but I was overly optimistic: headers are probably not going away.

Nadrieril avatar Mar 18 '20 00:03 Nadrieril

@Nadrieril What type of a lift would this look like in order to complete this feature. I'm looking to use dhall for github package management in some of my private repositories, but am unable to do so due to the inability to use auth headers.

This is what my .dhall file looks like, and what's specifically erroring out is the using(...headers) portion

let Package =
    { name : Text, version : Text, repo : Text, dependencies : List Text }

let packages = https:api.github.com/repos/my-org/my-repo/releases/assets/asset-id
  using (
    toMap { 
      Authorization = "${auth_token}", 
      Accept = "application/octet-stream", 
      User-Agent = "dhall" 
    }
  ): List Package

in packages

I was using the code below, and thought I was crazy at why dhall --file <filename> was able to execute the code but serde_dhall was not until I came across this issue.

fn read_package_set(&mut self, package_set_file: &Path) -> Result<()> {
        self.package_set = PackageSet::new(
            serde_dhall::from_file(package_set_file)
                .static_type_annotation()
                .parse()
                .context("Failed to parse the package set file")?,
        );
        Ok(())
    }

If you don't see this being addressed anytime soon, are there any potential workarounds or recommendations you might make to hack around it in the meantime?

ByronBecker avatar Feb 02 '22 00:02 ByronBecker

Well, the main issue is that I'm not really working on dhall-rust anymore. @basile-henry or @kryptn might you be motivated? The way I see it, implementing the full feature is too much effort atm since it might go away. A good alternative would be to add an option to dhall-rust to tell it which headers to use for each website. It would then just ignore the using keyword and use that instead. The API could look something like:

serde_dhall::from_file(filename)
    .add_header("api.github.com", "Accept", "application/octet-stream")
    .parse();

or take a hashmap or whatever.

I'm afraid the only workaround doable now would be to fork serde_dhall locally and hardcode the headers. If you want to try, the HTTP request is done there: https://github.com/Nadrieril/dhall-rust/blob/190ef2eb260754ffb43dfca42b28ac73986a68b6/dhall/src/semantics/resolve/resolve.rs#L298 You'll need to tweak the reqwest request to make it send the right headers.

Nadrieril avatar Feb 02 '22 15:02 Nadrieril

I actually think v21.0.0 of the standard adds this feature where a map of headers that can be specified via the DHALL_HEADERS environment variable. So maybe this is something we could support even though we don't support using?

I had a look at supporting 21.0 a few weeks ago, and I might come back to it at some point. But I have no timeline for it as I am pretty busy these days :sweat_smile:

basile-henry avatar Feb 02 '22 17:02 basile-henry

For reference I'm pretty new to both Dhall and Rust, so I appreciate both of your quick responses in helping to unblock me and find a solution.

@Nadrieril Thanks for the immediate unblocking suggestion, I'll look into that.

@basile-henry v21.0.0 looks like a cleaner standard organization wise imo, that works for me as a long term solution.

ByronBecker avatar Feb 02 '22 19:02 ByronBecker

Ohh cool I didn't follow that DHALL_HEADERS got merged, this is definitely the way to go then

Nadrieril avatar Feb 07 '22 23:02 Nadrieril