Skip to content

Use local files on connection failure, closes #298 #299

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

Merged
merged 6 commits into from
Oct 2, 2020

Conversation

WorldSEnder
Copy link
Contributor

@WorldSEnder WorldSEnder commented Oct 2, 2020

Use already downloaded tools when fetching latest release metadata fails. This adds a small cache file in the globalStoragePath containing the last fetched information.

Does not currently implement the feature as requested in #281 as I'm not certain what exactly such a flag should do (pick one):

  • Not download anything at all
  • "haskell.hlsUpdateBehavior": "keep-up-to-date" Try to fetch latest release information, if that fails fall back to local cache (what is done with this PR automatically)
  • "haskell.hlsUpdateBehavior": "prompt" Try to fetch latest release information, but not upgrade, even if it succeeds and instead display a prompt to update (probably sensible), still download artifacts for the proper ghc version, if necessary.
  • "haskell.hlsUpdateBehavior": "never-check" Don't check for latest release information, and do not upgrade, but still download artifacts for missing ghc versions, if needed.

I would want to implement #281 in this PR if that behavior is cleared up for me :)

@WorldSEnder
Copy link
Contributor Author

Ping @Meowcolm024 @runeksvendsen I would love to hear your feedback on this if you have time

@jneira jneira requested a review from lukel97 October 2, 2020 06:47
@jneira
Copy link
Member

jneira commented Oct 2, 2020

Very much thanks for the pr, it will improve the user experience a lot.
The last one (display a prompt) seems to be more complete and user friendly, similar to how vscode itself does the updates.
I would not ask if no version is available cause it would make the extension fail to load any haskell content.
However second and third are complementary, right? Prompt if it is succesful and fall back to previous version automatically if it is not (with a specific waning).

@WorldSEnder
Copy link
Contributor Author

The difference between the second and the third option is that the former is implemented, the latter would require some extra work - which I'm willing to put in, in any case.

@WorldSEnder
Copy link
Contributor Author

WorldSEnder commented Oct 2, 2020

Another point: I'm reading #272 and it seems the current code could use some validation of the json result returned from github, since it, as written, now writes a potentially malformed result to disk. I first thought to maybe use a simple json schema to validate, but after researching the vscode environment, it seems one would have to include rather big dependencies to do that.

Something like avj is quite a big dependency, a little bit smaller io-ts, which at least doesn't have any transitive dependencies. Then there's two very small and also small but probably not well maintained libraries. Honestly, I'd rather just write the logic directly inline. Any opinions on that?

more light-weight than using a fully fledged library like avj,
and most likely more than sufficient to handle edge cases.
@jneira
Copy link
Member

jneira commented Oct 2, 2020

I would say that inline would be better if the code is small enough, but I am not an expert in js/ts

@WorldSEnder
Copy link
Contributor Author

Alright, this should now also address #272 (by doing validation on the data received from github) and #281 (by adding a configuration item that can be set to prompt before deciding to use a newer version)

Shouldn't save too much bandwidth, but why not?

Also fix default option
@lukel97
Copy link
Collaborator

lukel97 commented Oct 2, 2020

Sounds like 5158096 also fixes #281, I'm happy with this design. This all LGTM, will quickly test it out locally

@lukel97
Copy link
Collaborator

lukel97 commented Oct 2, 2020

Thanks for doing this, looks great

@lukel97 lukel97 merged commit f09ce86 into haskell:master Oct 2, 2020
@jneira jneira linked an issue Oct 2, 2020 that may be closed by this pull request
@WorldSEnder WorldSEnder deleted the connection-issues branch October 3, 2020 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants