WIP: load SentryOptions from external sources. #536
Conversation
| if (shutdownTimeout != null) { | ||
| options.setShutdownTimeout(shutdownTimeout); | ||
| } | ||
|
|
There was a problem hiding this comment.
Here we will need to set all the non-null properties from external config
| */ | ||
| public static boolean isAvailable() { | ||
| try { | ||
| Class.forName("javax.naming.InitialContext", false, JndiSupport.class.getClassLoader()); |
There was a problem hiding this comment.
This isnt' very Android friendly.
We'd need a clear way to init without going through all these Java ways
There was a problem hiding this comment.
I'd say that none of the providers should be "executed" on Android, but just its own AndroidProvider.
it doesnt make sense to try to read system envs, locate the file etc if the only thing that makes sense for android is its own provider.
ideally, the options have a list of providers, and its empty by default, so depending on the integration, they add their own providers that make sense, something like that.
eg sentry-spring would have a system env, file system provider etc...
sentry-android would have only its own provider, that reads from the manifest
we'll also need a new module/package like io.sentry.config that has non android related things, I dont want customers needing to add extra rules to not fail their build because of unknown classes, this happened with the older version of the SDK and a lot of people complained.
| import javax.naming.Context; | ||
| import javax.naming.InitialContext; | ||
| import javax.naming.NamingException; | ||
| import javax.naming.NoInitialContextException; |
There was a problem hiding this comment.
this cant live in the sentry-core package.
Android SDK has a subset of the Java SDK, javax is not one of them
| return null; | ||
| } | ||
|
|
||
| try (InputStreamReader rdr = new InputStreamReader(is, charset)) { |
There was a problem hiding this comment.
does it make sense to use a BufferedReader as well?
馃摙 Type of change
馃摐 Description
The point of this PR at this stage is to discuss and think if this is the direction we want to go, or are there better ideas.
Adds the ability to load
SentryOptionsfrom external sources:sentry.propertiesfileThis is still WIP with missing:
@NotNulletc)馃挕 Motivation and Context
This feature is being brought in for the sake of compatibility with 1.x branch. Most of the code is either copied or inspired by the same functionality from 1.x branch with certain changes:
ILogger.What is supported:
sentry.propertiesSENTRY_PROPERTIES_FILEsentry.properties.fileSentryOptionsProvideris an entry-point to resolveSentryOptions. We can either use it inside no-argSentry#init()method, or resolve options and pass it toSentry#init(options).馃挌 How did you test it?
TODO
馃摑 Checklist
馃敭 Next steps