Mark HTTP support as deprecated to be removed in a future release. #790
Conversation
|
Please also see the checkstyle errors |
There was a problem hiding this comment.
In addition, we need to log warning if the HTTP protocol is chosen via the property we provide in the SDK, this way the user will see the warning in their logs. Users will not see a warning in code due to these deprecation annotations because this code is not used directly by the user, we have a builder that handles which instance of the client to build.
I agree but see above, we don't have a logging façade dependency / logging in place in the client; do we want to add a new dependency like slf4j for that? |
Signed-off-by: John Ewart <john@johnewart.net>
johnewart
force-pushed
the
deprecate-http
branch
from
aca0121 to
fdaca3a
Compare
Signed-off-by: John Ewart <john@johnewart.net>
|
Fixed checkstyle errors, removed one of the depreciation tags as requested and also added signoff -- amended and force pushed to keep things simpler. |
|
Created proposal #794 to track this |
|
I responded. I see 2 options: SLF4J or System.out.Println() because this is just for giving warnings for deprecation and not for other use cases (yet). |
Signed-off-by: John Ewart <john@johnewart.net>
| @@ -114,6 +117,7 @@ public class DaprClientHttp extends AbstractDaprClient { | |||
| this.client = client; | |||
| this.isObjectSerializerDefault = objectSerializer.getClass() == DefaultObjectSerializer.class; | |||
| this.isStateSerializerDefault = stateSerializer.getClass() == DefaultObjectSerializer.class; | |||
| System.out.println("NOTE: DaprClientHttp is deprecated and will be removed in a future release."); | |||
There was a problem hiding this comment.
NIT, but it might be more noticeable if we change "NOTE" to "deprecation ALERT" or something to the effect that holds attention as a warning that shouldn't be ignored
Codecov Report
@@ Coverage Diff @@
## master #790 +/- ##
============================================
- Coverage 77.62% 76.75% -0.87%
+ Complexity 1161 1146 -15
============================================
Files 105 104 -1
Lines 3647 3627 -20
Branches 419 418 -1
============================================
- Hits 2831 2784 -47
- Misses 603 636 +33
+ Partials 213 207 -6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@artursouza Can this be merged? |
|
We need to emit a warning because HTTP client can be used without any code change. Just via an env car, for example. |
|
Closing this in favor of #824 |
Description
In conversation with @artursouza earlier today regarding issue #763 we came to the conclusion that it might just make sense to simply deprecate the HTTP transport and let people implement their own HTTP client in the cases where they don't want to use gRPC. This would eliminate the split-dependency issue and (I believe) also make the JVM transition easier.
This PR marks the HTTP-related client constructors, constants and references as deprecated (at least I think I got all the places we instantiate HTTP transport related bits)
Issue reference
This is related to (and should be able to close) issue #763 -- also related to proposal #794
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: