add TrafficPolicy, remap Policy -> TrafficPolicy #51
Conversation
TheConcierge
force-pushed
the
ngrok/ryan/traffic-policy
branch
from
2d010fb
to
4c77b46
Compare
TheConcierge
force-pushed
the
ngrok/ryan/traffic-policy
branch
from
4c77b46
to
18e3196
Compare
TheConcierge
changed the title
TheConcierge
changed the title
TheConcierge
requested review from
andrewryno,
nikolay-ngrok,
rkolavo and
masonj5n
| @@ -580,7 +588,11 @@ impl<'local> NativeSessionRsImpl<'local> { | |||
| bldr.proxy_proto(ProxyProto::from(jeb.get_proxy_proto_version(self.env))); | |||
|
|
|||
| if let Some(policy) = jeb.get_policy(self.env).of_string(self.env) { | |||
There was a problem hiding this comment.
shouldn't we just drop this, it is still just the traffic_policy right?
There was a problem hiding this comment.
uhhhh.....
yeah i think we could. I was trying to cover for the case where they set the member variable directly, but I forgot that Java has strong encapsulation, so these are only available through getters and setters. Which means we shooooould be able to remove it?
But if it's truly doing nothing, we can also just leave it and rip it out when we actually do the real deprecation of policy at some point in...the next few months?
@nikolay-ngrok if you're ok leaving it until then then we can. If you'd rather it be removed now, lmk.
There was a problem hiding this comment.
🤷 happy to go either way 👍
Why
Currently, we are in the process of erasing strict policy types from our SDKs. This changes makes sure policy is sent to the backend as a traffic_policy string as opposed to a strictly formatted policy struct.
How
The traffic policy field already exists and functions in the underlying rust SDK. So all we have to do is plumb the new field through to Rust. Additionally, instances of Policy are remapped to traffic policy.
Validation
Current unit tests continue to run.
An additional unit test was added using the new TrafficPolicy field.
Additional Changes
Rust LSP
To make development easier, I added the rust SDK to our nix flake.
Bumping glibc to 2.28 in workflows
In our CI, we target/build against older versions of glibc to support customers on very old Linux. However, the checkout action provided by github has recently upgraded to Node 20, which needs a newer version than what we were targeting. For now, we are simply updating our version. However, if we end up needing to target older than 2.8, we will have to do a special workaround, as actions/checkout won't work without newer glibc versions.
Context: actions/checkout#1809