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

Request: In case of CLI method should be CLI too. #194

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

janbarasek
Copy link
Contributor

  • new feature
  • BC break? yes

In the case of obtaining a service for an Http Request in CLI mode, it does not make sense for the call method to be GET when no request exists.

I think Request should either throw an exception that cannot be used in a CLI context, or it should return a special CLI method.

Current behavior looks illogical:

Screenshot from 2021-03-01 10-06-40

Thanks.

@dg
Copy link
Member

dg commented Mar 1, 2021

HTTP method CLI doesn't exist.

@janbarasek
Copy link
Contributor Author

I don't know how to better represent this (if at all).

When obtaining the current Request from the DI, it should be possible to verify that the Request was created artificially in CLI mode and is not a real request. In that case, I can't rely on a number of things, such as URLs.

What do you think?

@milo
Copy link
Member

milo commented Mar 1, 2021

In a strict way - an HTTP request can be automatically created only via HTTP protocol so exception is more accurate. In practice - I'm not sure.

@dg dg force-pushed the master branch 10 times, most recently from 5ace0ce to 3affe3a Compare March 4, 2021 20:04
@dg dg force-pushed the master branch 4 times, most recently from 6ee31b8 to 4f4a403 Compare April 27, 2021 21:33
@dg dg force-pushed the master branch 4 times, most recently from a8895b6 to 9409a5f Compare August 25, 2021 15:26
@dg dg force-pushed the master branch 5 times, most recently from 7b7f9ff to 78bee95 Compare September 3, 2021 22:02
@dg dg force-pushed the master branch 6 times, most recently from da24b94 to 540335c Compare March 20, 2023 13:43
@dg dg force-pushed the master branch 2 times, most recently from e7c7e2d to bf945f3 Compare August 5, 2023 19:08
@dg dg force-pushed the master branch 3 times, most recently from 9a14e6e to a20fb8f Compare November 14, 2023 18:31
@dg dg force-pushed the master branch 5 times, most recently from 55488bd to 2042d2e Compare December 11, 2023 13:01
@dg dg force-pushed the master branch 2 times, most recently from 4960652 to 5e67add Compare May 2, 2024 10:56
@dg dg force-pushed the master branch 5 times, most recently from 689f4ae to 33aae19 Compare November 5, 2024 06:45
@dg dg force-pushed the master branch 4 times, most recently from 09923de to 02ae846 Compare January 16, 2025 04:45
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