Skip to content

Comments

Simplify use of reqwest a bit#202

Merged
hansl merged 2 commits intomasterfrom
johnw/reqwest
Nov 22, 2019
Merged

Simplify use of reqwest a bit#202
hansl merged 2 commits intomasterfrom
johnw/reqwest

Conversation

@jwiegley
Copy link
Contributor

No description provided.

@jwiegley jwiegley requested a review from a team as a code owner November 22, 2019 19:45
@jwiegley jwiegley self-assigned this Nov 22, 2019
@ghost
Copy link

ghost commented Nov 22, 2019

We currently get a bare Reqwest error if you use dfx when the client isn't running:

» dfx canister call hello '("Stanley")'
An error occured:
Reqwest(
    Error(
        Hyper(
            Error(
                Connect,
                Os {
                    code: 61,
                    kind: ConnectionRefused,
                    message: "Connection refused",
                },
            ),
        ),
        "http://127.0.0.1:8000/api/v1/submit",
    ),
)

Should I file this as a separate concern? Or does it make sense to throw in here, since you're already touched Reqwest error messages?

@lsgunnlsgunn
Copy link
Contributor

lsgunnlsgunn commented Nov 22, 2019

My .02, please wrap this is a nicer error message, if possible. To me and to at least one member of the community "Reqwest" looks like an Elmer Fudd impression (AKA, a typo).

(I have learned that is it legit, but it looks sort of unprofessional if you don't know that.)

@jwiegley
Copy link
Contributor Author

@stanleygjones Improved error handling is much better split off as a separate concern, since I don't have the time to make that fix, and I doubt @hansl will want to work on top of my PR.

@hansl
Copy link
Contributor

hansl commented Nov 22, 2019

We have a separate ticket for improved error handling.

Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this as it's a good cleanup. Next step would be to have async/await.

@hansl hansl merged commit 7ee8cc4 into master Nov 22, 2019
@hansl hansl deleted the johnw/reqwest branch November 22, 2019 21:37
@jwiegley
Copy link
Contributor Author

Yes, async/await will make this module beautifully clear.

dfinity-bot added a commit that referenced this pull request Jul 15, 2020
## Changelog for common:
Branch: master
Commits: [dfinity-lab/common@05c44edd...9162ebde](https://github.com/dfinity-lab/common/compare/05c44eddb864464c06c27295460eabcf923bc2e9...9162ebdee5e914c0a7d22c71596d1760230156df)

* [`2a5d4183`](https://github.com/dfinity-lab/common/commit/2a5d4183065f7811edce500e9d20fd5a4fcb10df) niv nixpkgs: update e79d1e83 -> 5a462920
* [`413ce2b3`](https://github.com/dfinity-lab/common/commit/413ce2b3b176863ff33c4fdf276842d7f94bbb5b) rust: 1.41.1 -> 1.43
* [`899366be`](https://github.com/dfinity-lab/common/commit/899366be97d4c79fdcf4230164a7ffca9ed4e309) niv nixpkgs: update 5a462920 -> 05a32d8e
* [`f2c71849`](https://github.com/dfinity-lab/common/commit/f2c7184973a5bd3d4dacfb0c1245d07ef8e39139) Add support for incremental Rust workspaces ([dfinity-lab/common⁠#199](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/199))
* [`f484c11e`](https://github.com/dfinity-lab/common/commit/f484c11e040c2a91204cfddc8ddaca1c5e51979d) Revert "Add support for incremental Rust workspaces ([dfinity-lab/common⁠#199](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/199))" ([dfinity-lab/common⁠#204](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/204))
* [`74c64048`](https://github.com/dfinity-lab/common/commit/74c640486d2e34d3cc63df1a20e4346e07a21bbf) gitSource.nix: Use gitMinimal
* [`fa568e26`](https://github.com/dfinity-lab/common/commit/fa568e267a189ee51721af6d7d08eb2af8d54de4) disable parallel rustc build ([dfinity-lab/common⁠#206](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/206))
* [`90627152`](https://github.com/dfinity-lab/common/commit/9062715209261439648898b03f7652cd3f138399) [INF-1316] Support for overriding cargoBuild ([dfinity-lab/common⁠#207](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/207))
* [`e2d48d84`](https://github.com/dfinity-lab/common/commit/e2d48d8404ccce8252d9a78d76282fc9f1bd026b) nixpkgs: Backports patches for static builds of Haskell ([dfinity-lab/common⁠#208](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/208))
* [`45bbce1c`](https://github.com/dfinity-lab/common/commit/45bbce1cfb34c6b723bbe3f89466a63ae66bfb77) rust: Add release build with symbols ([dfinity-lab/common⁠#209](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/209))
* [`c9a4327d`](https://github.com/dfinity-lab/common/commit/c9a4327df3040921fa27a7b48c6dc924b13a423b) ci: introduce the all-systems-go aggregate job
* [`e8689e50`](https://github.com/dfinity-lab/common/commit/e8689e50dfff33663f498df2baf994eeee39bb7d) Disable selectors2 tests on Darwin
* [`eb8225ea`](https://github.com/dfinity-lab/common/commit/eb8225ea65dfa4bcc688a3d58f7c9633affd9234) nix/overlays/ci.nix: increase schedulingPriority of all-systems-go
* [`dac9442c`](https://github.com/dfinity-lab/common/commit/dac9442c1110253f3386208790757ec11ff4605c) [INF-589] Incremental build support ([dfinity-lab/common⁠#213](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/213))
* [`ebaf4cf7`](https://github.com/dfinity-lab/common/commit/ebaf4cf726e75f2037570f162f79b103a3931bbd) make rustWorkspace overridable
* [`a1fb28aa`](https://github.com/dfinity-lab/common/commit/a1fb28aa2d8ddb6ac730264dc4273c4643b818f7) allow crates-io references for existing naersk builds
* [`724330e8`](https://github.com/dfinity-lab/common/commit/724330e8e6d8db0fdc6d8b835ed7b107333fed44) Update niv-updater-action to v6
* [`b48fa5d6`](https://github.com/dfinity-lab/common/commit/b48fa5d64ae8aecd4b900f4e04054c942f839e38) Fix typo in README
* [`a63e292f`](https://github.com/dfinity-lab/common/commit/a63e292f45cf2d5238b04fb2a2bc173ea44ba21b) Add a replaceStdenv function ([dfinity-lab/common⁠#202](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/202))
* [`7d3742a2`](https://github.com/dfinity-lab/common/commit/7d3742a2efb70206dac5dcfba78f7102cc2e0b2a) niv cargo2nix: update b9ef68fe -> 31d34998
* [`8431c1e6`](https://github.com/dfinity-lab/common/commit/8431c1e66239f7bc14a0f113c95692648bb211d2) patch ruamel
* [`9162ebde`](https://github.com/dfinity-lab/common/commit/9162ebdee5e914c0a7d22c71596d1760230156df) Migrate back to OpenSSL ([dfinity-lab/common⁠#219](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/219))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants