Fix tests without with-http flag
#1159
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Makes the `Status` type more general; previously support for
`Network.HTTP.Client` was hardcoded. In short:
```
data Status = Status
{ _stack :: NonEmpty Chained
[...]
-- , _manager :: Maybe Dynamic
-- -- importing the same expression twice with different values
++ , _remote :: URL -> StateT Status IO Data.Text.Text
++ -- ^ The remote resolver, fetches the content at the given URL.
[...]
}
```
`toHeaders` will be needed for mock http testing
Implements a mock http client that handles requests to: - `https://raw.githubusercontent.com/dhall-lang/dhall-lang/master/` - `https://test.dhall-lang.org/Bool/package.dhall` - `https://httpbin.org/user-agent` This allows tests involving remote imports to succeed even when compiled without the `with-http` flag.
... to prevent regressions from occurring in the future
Gabriella439
approved these changes
Jul 27, 2019
Gabriella439
added a commit
that referenced
this pull request
Nov 8, 2019
Related to #1511 This fixes a performance regression introduced in #1159 where `newManager` was being called on every remote import. This fixes that by going back to caching the `Manager` created by the first request. This leads to *dramatic* performance improvements for import-rich packages (like the Prelude or `dhall-kubernetes`) on the first import. For example, here are the performance numbers for importing the Prelude for a cold cache before and after this change: Before: ``` $ XDG_CACHE_HOME=.cache time dhall hash <<< 'https://prelude.dhall-lang.org/package.dhall' sha256:99462c205117931c0919f155a6046aec140c70fb8876d208c7c77027ab19c2fa 64.10 real 10.83 user 2.73 sys ``` After: ``` $ XDG_CACHE_HOME=.cache2 time dhall hash <<< 'https://prelude.dhall-lang.org/package.dhall' sha256:99462c205117931c0919f155a6046aec140c70fb8876d208c7c77027ab19c2fa 4.39 real 0.49 user 0.15 sys ``` That's ~16x faster! The improvement for `dhall-kubernetes` is smaller, but still significant: Before: ``` $ XDG_CACHE_HOME=.cache3 time dhall hash <<< ~/proj/dhall-kubernetes-charts/stable/jenkins/index.dhall sha256:04ebd960f6af331c49c3ccaedb353ac8269032b54fe0a29bd167febcd7104d4f 833.59 real 145.36 user 36.16 sys After: ``` $ XDG_CACHE_HOME=.cache4 time dhall hash <<< ~/proj/dhall-kubernetes-charts/stable/jenkins/index.dhall sha256:04ebd960f6af331c49c3ccaedb353ac8269032b54fe0a29bd167febcd7104d4f 381.41 real 8.41 user 1.91 sys ``` ... or ~2-3x improvement.
mergify bot
pushed a commit
that referenced
this pull request
Nov 8, 2019
* Fix import resolution performance regression Related to #1511 This fixes a performance regression introduced in #1159 where `newManager` was being called on every remote import. This fixes that by going back to caching the `Manager` created by the first request. This leads to *dramatic* performance improvements for import-rich packages (like the Prelude or `dhall-kubernetes`) on the first import. For example, here are the performance numbers for importing the Prelude for a cold cache before and after this change: Before: ``` $ XDG_CACHE_HOME=.cache time dhall hash <<< 'https://prelude.dhall-lang.org/package.dhall' sha256:99462c205117931c0919f155a6046aec140c70fb8876d208c7c77027ab19c2fa 64.10 real 10.83 user 2.73 sys ``` After: ``` $ XDG_CACHE_HOME=.cache2 time dhall hash <<< 'https://prelude.dhall-lang.org/package.dhall' sha256:99462c205117931c0919f155a6046aec140c70fb8876d208c7c77027ab19c2fa 4.39 real 0.49 user 0.15 sys ``` That's ~16x faster! The improvement for `dhall-kubernetes` is smaller, but still significant: Before: ``` $ XDG_CACHE_HOME=.cache3 time dhall hash <<< ~/proj/dhall-kubernetes-charts/stable/jenkins/index.dhall sha256:04ebd960f6af331c49c3ccaedb353ac8269032b54fe0a29bd167febcd7104d4f 833.59 real 145.36 user 36.16 sys After: ``` $ XDG_CACHE_HOME=.cache4 time dhall hash <<< ~/proj/dhall-kubernetes-charts/stable/jenkins/index.dhall sha256:04ebd960f6af331c49c3ccaedb353ac8269032b54fe0a29bd167febcd7104d4f 381.41 real 8.41 user 1.91 sys ``` ... or ~2-3x improvement. * Fix `-f-with-http` build * Remove unnecessary `CPP` ... as caught by @sjakobi
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: This PR includes the changes in PR #1157.
Implements a mock http client that handles requests to:
https://raw.githubusercontent.com/dhall-lang/dhall-lang/master/https://test.dhall-lang.org/Bool/package.dhallhttps://httpbin.org/user-agentThis allows tests involving remote imports to succeed even when compiled
without the
with-httpflag. Addresses #1135.