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

Support custom import headers #134

Open
Nadrieril opened this issue Mar 4, 2020 · 6 comments
Open

Support custom import headers #134

Nadrieril opened this issue Mar 4, 2020 · 6 comments
Labels
standard-compliance Something doesn't work according to the dhall standard
Milestone

Comments

@Nadrieril
Copy link
Owner

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

@Nadrieril Nadrieril added postponed This issue is low priority standard-compliance Something doesn't work according to the dhall standard labels Mar 4, 2020
@Nadrieril Nadrieril added this to the v1.0 milestone Mar 18, 2020
@Nadrieril
Copy link
Owner Author

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

@Nadrieril Nadrieril removed the postponed This issue is low priority label Mar 18, 2020
@ByronBecker
Copy link

ByronBecker commented Feb 2, 2022

@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?

@Nadrieril
Copy link
Owner Author

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:

Ok(reqwest::blocking::get(url).unwrap().text().unwrap())

You'll need to tweak the reqwest request to make it send the right headers.

@basile-henry
Copy link
Collaborator

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 😅

@ByronBecker
Copy link

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.

@Nadrieril
Copy link
Owner Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard-compliance Something doesn't work according to the dhall standard
Projects
None yet
Development

No branches or pull requests

3 participants