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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch exceptions whilst trying to fetch metadata #325

Merged
merged 3 commits into from Jul 19, 2019

Conversation

@carlosdagos
Copy link
Contributor

commented Jul 15, 2019

Description of the change

The previous way was only handling the case where the metadata fetched
could not be correctly parsed. Now it will also disable the global
cache if the HTTP request fails for any reason.

NOTE

I don't know if I'm on to the right thing here. In my build environment
the metadata URL isn't reachable due to firewalls (and I can't change that
in the short term, unfortunately). Maybe this isn't the right change and
I should open an issue to track all issues related to my use case 馃槃

Thanks for spago! I'm really enjoying using it 馃憤

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 馃槉

@carlosdagos carlosdagos force-pushed the carlosdagos:topic/http-exception branch from 07458d7 to 87ef717 Jul 15, 2019

@f-f

f-f approved these changes Jul 16, 2019

Copy link
Member

left a comment

Hi @carlosdagos, thanks for this! Looks good already, I just left a comment in case you'd wish to use the metadata too 馃檪

pure meta
pure mempty)
(do
metaBS <- Http.getResponseBody `fmap` Http.httpBS metaURL

This comment has been minimized.

Copy link
@f-f

f-f Jul 16, 2019

Member

If you'd like to be able to customize the location of the metadata (because e.g. you have a proxy to GitHub in your network, or you could download it locally, etc) we could try to read it from environment, e.g. something like:

Suggested change
metaBS <- Http.getResponseBody `fmap` Http.httpBS metaURL
metaBS <- Dhall.input Dhall.lazyText ("env:SPAGO_METADATA_JSON ? " <> metaURL <> " as Text")

This won't compile because types mismatch but just to give you an idea. This Dhall string:

  • tries to read the SPAGO_METADATA_JSON variable from environment
  • if that's missing, it tries to import the default URL as Text
  • imports the result as a lazy Text, so then we can parse it

When setting the SPAGO_METADATA_JSON variable, you could put there something like https://some.proxy/metadata.json as Text or ./local/metadata.json as Text or even "{ \"some\": \"escaped json\" }"

This comment has been minimized.

Copy link
@carlosdagos

carlosdagos Jul 16, 2019

Author Contributor

I really like this idea. Will experiment with it a bit 馃憤

Show resolved Hide resolved src/Spago/GlobalCache.hs Outdated

carlosdagos added some commits Jul 15, 2019

Catch exceptions whilst trying to fetch metadata
The previous way was only handling the case where the metadata fetched
could not be correctly parsed. Now it will also disable the global
cache if the HTTP request fails for any reason.

@carlosdagos carlosdagos force-pushed the carlosdagos:topic/http-exception branch from 4fb9a19 to cc04d53 Jul 16, 2019

@f-f

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

@carlosdagos I'll merge this before it gets too out of sync with master, feel free to open another PR to add support for overriding the metadata URL 馃檪

Thank you!

@f-f f-f merged commit 70d5d5c into spacchetti:master Jul 19, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@carlosdagos carlosdagos deleted the carlosdagos:topic/http-exception branch Jul 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.