search

Brightspace / D2L.Security.OAuth2

Brightspace OAuth 2.0 for C#
Apache License 2.0
7 stars 16 forks source link

Added MonoRsaSecurityKey to allow use of library within mono #64

Closed drose-d2l closed 7 years ago

omsmith commented 7 years ago

Any idea on how to get this tested in CI?

j3parker commented 7 years ago

AppVeyor has mono installed: https://www.appveyor.com/docs/build-environment/#mono

It's Windows but I'd be satisfied if we added it to the build+test matrix.

drose-d2l commented 7 years ago

changes request should be done now

omsmith commented 7 years ago

Code changes look okay at this point.

@drose-d2l are you able to figure out building/testing with mono via build matrix in our appveyor config? Environment comes with Mono 4.0.2 SR2.

drose-d2l commented 7 years ago

So after looking, I can honestly say I have no idea. The documentation for Appveyor does not seem to be the greatest when it comes to how to build multiple targets. If either you or @j3parker have an idea then I would love the assistance haha.

drose-d2l commented 7 years ago

bump

j3parker commented 7 years ago

I haven't done it before but I'll try in another branch.

j3parker commented 7 years ago

I just noticed that libgit2sharp builds with AppVeyor for Windows and Travis CI for Linux+Mono, which makes sense to me.

@drose-d2l if I enabled Travis for this repo would you be willing to investigate getting it building there (via a different PR with a travis config)?

j3parker commented 7 years ago

https://travis-ci.org/Brightspace/D2L.Security.OAuth2

drose-d2l commented 7 years ago

Sure I can do that, we have built a few mono packages in travis already so it shouldn't take too long. Would you need me to deploy it to nuget as well?

The other option is I can modify the compiler directives to use a runtime check

Type.GetType ("Mono.Runtime") != null;

this will also work, but does incur slightly more overhead. The nice thing is it will allow us to only need one assembly.

j3parker commented 7 years ago

Hm, so we currently publish manually (urg.)

That makes me realize: if we did Travis and AppVeyor builds we'd presumably have to combine the build artifacts. I assume mono DLLs get their own folder in the nupkg?

That's not so bad... would be a good excuse to finally automate the publishing.

I'm inclined to not have the runtime check but I might be convinced that it's not worth the build hassle to avoid it. @omsmith ?

omsmith commented 7 years ago

I'd say it depends on just how much overhead. The locations of the checks don't seem terribly in the way, only thing I'd really expect it to be in the path of that we "do often" is remote jwks -> d2lsecuritykey(s). I don't know what are ops/s is on that in our production workload though.

@drose-d2l want to run a before/after of the benchmark suite, without changes and with runtime checks and see what you get? We can decide from there I suppose?

omsmith commented 7 years ago

Double-checking, the benchmarks aren't really going to exercise what would slow down, because they only do the jwk->security key once, so, probably a pointless exercise really.

omsmith commented 7 years ago

Closing due to inactivity