-
Notifications
You must be signed in to change notification settings - Fork 71
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
Yggdrasil engine #256
base: main
Are you sure you want to change the base?
Yggdrasil engine #256
Changes from 17 commits
f84b0b1
ba5370c
162d408
48811ed
9a4a8ae
09ed04b
3862610
c736c2b
8baf2c6
7674b6d
ddadd4d
8517b43
4ba1c45
d83d62d
2ec31e5
0420d5c
629636e
2aec091
12b5834
2a2e090
979229f
d49d244
cfefa56
76b99fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,17 +107,6 @@ public void disableAllExcept(String... excludedFeatures) { | |
} | ||
} | ||
|
||
@Override | ||
public Variant deprecatedGetVariant(String toggleName, UnleashContext context) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a public interface change. This was in place to support legacy evaluation of variants. We're removing that in the next major, as per #234 |
||
return null; | ||
} | ||
|
||
@Override | ||
public Variant deprecatedGetVariant( | ||
String toggleName, UnleashContext context, Variant defaultValue) { | ||
return null; | ||
} | ||
|
||
public void resetAll() { | ||
disableAll = false; | ||
enableAll = false; | ||
|
@@ -176,15 +165,5 @@ public List<EvaluatedToggle> evaluateAllToggles(@Nullable UnleashContext context | |
getVariant(toggleName))) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
@Override | ||
public void count(String toggleName, boolean enabled) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Public interface change. I don't see a reason why anyone would be using this but it is public. The Yggdrasil engine is now the sink for this data |
||
// Nothing to count | ||
} | ||
|
||
@Override | ||
public void countVariant(String toggleName, String variantName) { | ||
// Nothing to count | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
package io.getunleash.metric; | ||
|
||
import io.getunleash.engine.MetricsBucket; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its a bit sneaky but this was a drop in replacement for the existing metrics bucket. All I needed to do was include the new import and the code is unchanged |
||
import io.getunleash.event.UnleashEvent; | ||
import io.getunleash.event.UnleashSubscriber; | ||
import io.getunleash.lang.Nullable; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,12 +5,14 @@ | |
import io.getunleash.event.EventDispatcher; | ||
import io.getunleash.util.AtomicLongSerializer; | ||
import io.getunleash.util.DateTimeSerializer; | ||
import io.getunleash.util.InstantSerializer; | ||
import io.getunleash.util.UnleashConfig; | ||
import io.getunleash.util.UnleashURLs; | ||
import java.io.IOException; | ||
import java.io.OutputStreamWriter; | ||
import java.net.HttpURLConnection; | ||
import java.net.URL; | ||
import java.time.Instant; | ||
import java.time.LocalDateTime; | ||
import java.util.concurrent.atomic.AtomicLong; | ||
|
||
|
@@ -33,6 +35,7 @@ public DefaultHttpMetricsSender(UnleashConfig unleashConfig) { | |
this.gson = | ||
new GsonBuilder() | ||
.registerTypeAdapter(LocalDateTime.class, new DateTimeSerializer()) | ||
.registerTypeAdapter(Instant.class, new InstantSerializer()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yggdrasil deals in Instants not Datetimes. This allows us to serialize those to ISO 8601 when we send metrics |
||
.registerTypeAdapter(AtomicLong.class, new AtomicLongSerializer()) | ||
.create(); | ||
} | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package io.getunleash.util; | ||
|
||
import com.google.gson.*; | ||
import java.lang.reflect.Type; | ||
import java.time.Instant; | ||
|
||
public class InstantSerializer implements JsonSerializer<Instant>, JsonDeserializer<Instant> { | ||
|
||
@Override | ||
public JsonElement serialize( | ||
Instant instant, Type typeOfSrc, JsonSerializationContext context) { | ||
return new JsonPrimitive(instant.toString()); | ||
} | ||
|
||
@Override | ||
public Instant deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) | ||
throws JsonParseException { | ||
return Instant.parse(json.getAsString()); | ||
} | ||
} |
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.
Classifier needs to be fixed before releasing this but I think that's a chunk of work distinct enough to want its own PR