-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[Sonar] Fix invalid fetch query to sonarqube >=6.6 #6636
[Sonar] Fix invalid fetch query to sonarqube >=6.6 #6636
Conversation
|
Thanks for submitting this patch. Do you know of an example of a public sonar instance running >=6.6 that we could use as a test case for this? |
I don't think we have any existing tests for other instances (all mocked), but I can deploy an instance of this branch and test against such a SonarQube instance on the intranet. I'm wondering if we may be getting to a point where it'd be worth investigating some kind of sandbox env (preferably container based) where we can run a few instances of some of the self-hosted tools (or spin them up dynamically as needed for tests) |
Just in case you're looking for an instance for an one-time manually test: You can use a project, e.g. |
Beautiful, thank you @chkpnt! https://shields-staging-pr-6636.herokuapp.com/sonar/quality_gate/de.chkpnt:truststorebuilder-gradle-plugin?server=https%3A%2F%2Fsonar.chkpnt.de&sonarVersion=6.6 I'm going to assume the choice of "one-time manually" phrasing was very deliberate, but going to ask you a follow up question anyway 😄 We run our full suite of integration tests twice daily against (mostly) live targets of upstream services/platforms/etc. Is your SonarQube instance typically internet facing and anonymously accessible (or did you perhaps temporarily make it so for this thread?), and if so, would you be opposed to us using your SonarQube instance for one of the Sonar tests? That would mean a grand total of two http requests sent to your instance from us per day. The ability to target live instances with our test runs helps us catch issues like this more quickly when vendors change their API contracts, as Shields integrates with far too many services and platforms for our team to be up to date with breaking changes upstream. |
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.
Thanks!
// componentKey query param was renamed in version 6.6 | ||
const componentKey = | ||
parseFloat(sonarVersion) >= 6.6 ? 'component' : 'componentKey' | ||
qs = { | ||
componentKey: component, | ||
[componentKey]: component, |
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.
Long term it'd probably be better to refactor this out and repurpose isLegacyVersion
to have a single helper that provides the url, schema, and query string. However, this will work for now so happy to proceed to address the immediate blocking issue
Will soon be working again, see badges/shields#6636
@calebcartwright I just phrased it this way to not be blamed for breaking tests As you see in the size of my SonarQube and the size of the scanned projects, it's just a hobby instance for my own pet projects, so there is no HA (although it's normally running 24/7). But of course, if you're just looking for a real-world public readable instance, you are free to use it, as there is no difference in using it for the badges in my own READMEs. Just wanted to throw in another link: I knew the Apache Foundation were hosting it's own SonarQube instance... but it has certainly turned out, that they sunsetted their service. |
Will soon be working again, see badges/shields#6636
Closes #5882