-
Notifications
You must be signed in to change notification settings - Fork 89
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
Add user agent for log service client #363
Conversation
...gservice/src/main/java/org/apache/spark/streaming/aliyun/logservice/constants/Constants.java
Outdated
Show resolved
Hide resolved
/** | ||
* @author jiashuangkai | ||
* @version 1.0 | ||
* @since 2019-09-27 14:56 | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok have deleted
@@ -0,0 +1,41 @@ | |||
package org.apache.spark.streaming.aliyun.logservice.utils; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing license header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have added
import java.util.Properties; | ||
|
||
public class VersionInfoUtils { | ||
private static final String VERSION_INFO_FILE = "spark-versioninfo.properties"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to use "versioninfo.properties" instead of "spark-versioninfo.properties"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other sdk have this file: versioninfo.properties
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or you can use "emr-logservice-versioninfo.properties" instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
* @since 2019-09-27 14:56 | ||
*/ | ||
public interface Constants { | ||
String LOG_CONNECTOR_USER_AGENT = VersionInfoUtils.getDefaultUserAgent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any need to add this new class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many classes use this constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use "VersionInfoUtils.getDefaultUserAgent()" directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
Sign the CLA please |
LGTM,thanks |
No description provided.