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

WINDUP-1298: Really don't make network calls in offline #1072

Merged
merged 3 commits into from Feb 6, 2017

Conversation

jsight
Copy link
Member

@jsight jsight commented Feb 3, 2017

This is still a little weird, as this creates some validation errors on poms and things like that. I really wonder if we should just skip this rule entirely in offline.

@OndraZizka
Copy link
Contributor

Provided it doesn't fail the run, I think it's ok.
Maybe we could bundle more XSDs? E.g. for the pom

@jsight
Copy link
Member Author

jsight commented Feb 3, 2017

Yeah, I think bundling maven xsds would make sense.

@d-s
Copy link
Contributor

d-s commented Feb 3, 2017

Yeah, seems like we will get "XML File is not valid" hits on every app now. How was it making calls after the "return null;" before?

@jsight
Copy link
Member Author

jsight commented Feb 3, 2017

The xerces resolver would call this one and if it returned null, then it would try to resolve it itself if it looked like a URL. I think it treats "null" as an indication that a resolver doesn't know how to resolve a particular type rather than as an indication of failure.

I'm going from reading the implementation, though, rather than from reading docs as I haven't seen clear documentation on this.

@jsight
Copy link
Member Author

jsight commented Feb 3, 2017

It looks like this will break a few tests because of increasing the number of failures.

@d-s
Copy link
Contributor

d-s commented Feb 3, 2017

Don't really love spitting out clsf/hint everywhere that XML files are not valid when we didn't try to validate them. Kind of prefer your suggestion above, that we just not run this rule if we're offline.

@OndraZizka
Copy link
Contributor

So maybe catch the exception somewhere up and just print a log message?
INFO if offline, WARN if online

@mareknovotny
Copy link
Contributor

mareknovotny commented Feb 6, 2017

I think this should not be returning null it just gives a chance to others resolvers in chain.

We need to skip the rule completely.

@d-s
Copy link
Contributor

d-s commented Feb 6, 2017

looks good here

@jsight jsight merged commit 322d939 into windup:master Feb 6, 2017
@jsight jsight deleted the windup-1298 branch February 6, 2017 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants