Skip to content
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

Clean Up Reflection Warnings #48

Merged
merged 6 commits into from
Aug 25, 2021
Merged

Clean Up Reflection Warnings #48

merged 6 commits into from
Aug 25, 2021

Conversation

milt
Copy link
Member

@milt milt commented Aug 20, 2021

Per discussion on other PRs with @kelvinqian00 , lrs should not make decisions like the setting of the global var *warn-on-reflection* for implementers. There are situations where reflection might be needed, and the option to suppress the warnings without the need for careful require order is a must.

Remove all the direct sets for it and see if it can be put into user.clj or some similar. With deps + cli it is actually hard to do this in a way that works for everyone's repl. Adding it to the init namespaces of the in-memory dev LRS (and keeping it in bench) will have to do for now.

@milt milt marked this pull request as ready for review August 20, 2021 12:05
@milt
Copy link
Member Author

milt commented Aug 20, 2021

Upstream warnings that will remain until we update pedestal and a few other bits:

Reflection warning, io/pedestal/log.clj:686:26 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:688:16 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:692:20 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:695:20 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:735:17 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:737:17 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:740:19 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:742:19 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:772:21 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:774:21 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:798:19 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:800:26 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:801:36 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/log.clj:802:43 - reference to field span on io.opentracing.Scope can't be resolved.
Reflection warning, io/pedestal/http/route.clj:67:3 - call to static method decode on java.net.URLDecoder can't be resolved (argument types: unknown, java.lang.String).
Reflection warning, io/pedestal/http/route.clj:73:3 - call to static method encode on java.net.URLEncoder can't be resolved (argument types: unknown, java.lang.String).
Reflection warning, ring/util/codec.clj:95:5 - call to static method encode on java.net.URLEncoder can't be resolved (argument types: java.lang.String, unknown).
Reflection warning, ring/util/codec.clj:130:6 - call to static method decode on java.net.URLDecoder can't be resolved (argument types: java.lang.String, unknown).
Reflection warning, crypto/random.clj:28:12 - call to static method encodeHex on org.apache.commons.codec.binary.Hex can't be resolved (argument types: unknown).
Reflection warning, crypto/random.clj:28:3 - call to java.lang.String ctor can't be resolved.
Reflection warning, hiccup/util.clj:78:19 - call to static method encode on java.net.URLEncoder can't be resolved (argument types: java.lang.String, unknown).
Reflection warning, io/pedestal/http/jetty.clj:106:22 - call to org.eclipse.jetty.server.ServerConnector ctor can't be resolved.
Reflection warning, io/pedestal/http/jetty.clj:183:32 - call to org.eclipse.jetty.server.ServerConnector ctor can't be resolved.
Reflection warning, io/pedestal/http/jetty.clj:189:31 - call to org.eclipse.jetty.server.ServerConnector ctor can't be resolved.

@kelvinqian00
Copy link
Contributor

You still have a (set! *warn-on-reflection* true) line in bench.clj. Is that intentional?

@milt
Copy link
Member Author

milt commented Aug 20, 2021

Yep, it's on a dev path so I leave it alone.

Copy link
Contributor

@kelvinqian00 kelvinqian00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno why I thought CI was still failing (which is why I haven't approved it yet) but here you go!

@milt
Copy link
Member Author

milt commented Aug 25, 2021

Dunno why I thought CI was still failing (which is why I haven't approved it yet) but here you go!

As of the master merge 5 days ago, it's passing CI, am I missing something? There was actually a change merged in that improved the reliability of the conformance tests a good deal and this was an intermittent error, so I think it's good to go now.

@kelvinqian00
Copy link
Contributor

Since you mentioned that, I ran the conformance tests in lrsql and they work, so that's good.

@milt milt merged commit fd5e04a into master Aug 25, 2021
@milt milt deleted the remove_refl_warnings branch August 25, 2021 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants