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

Fix Add Datasource via existing driver Issue #941

Merged
merged 2 commits into from
Apr 20, 2017
Merged

Fix Add Datasource via existing driver Issue #941

merged 2 commits into from
Apr 20, 2017

Conversation

mtho11
Copy link
Contributor

@mtho11 mtho11 commented Apr 6, 2017

This fixes an issue where the JDBC Driver for existing drivers that was not pulling the data from the WildFly/EAP server but instead, incorrectly from defaults. So it would always try to create the datasource using the default name (like 'h2') instead of the custom name returned from the Wildfly server. Another issue, where username/password/securityDomain was empty it would send them as empty strings to the server instead of not sending them at all.

ds-add-fail mov

Links

Testing Instructions

As this is an asynchronous operation, the best way to test this is to monitor the logs of the WildFly/EAP server and make sure there are no errors during Datasource creation. Running the Hawkular Agent in debug logging mode helps if there are issues

@mtho11
Copy link
Contributor Author

mtho11 commented Apr 6, 2017

@miq-bot add_label middleware, bug, fine/yes

@miq-bot
Copy link
Member

miq-bot commented Apr 6, 2017

This pull request is not mergeable. Please rebase and repush.

@abonas
Copy link
Member

abonas commented Apr 6, 2017

If this is a BZ, please paste a link to it.
Also, it will be helpful to have a more detailed description - what was specifically the issue/bug fixed here?

@mtho11
Copy link
Contributor Author

mtho11 commented Apr 7, 2017

@miq-bot add_label angular dialogs

@mtho11
Copy link
Contributor Author

mtho11 commented Apr 10, 2017

@abonas yes, I don't usually link to BZs, screenshots, etc until it's not WIP.

@mtho11 mtho11 changed the title [WIP] Fix Add Datasource via existing driver Issue Fix Add Datasource via existing driver Issue Apr 10, 2017
@miq-bot miq-bot removed the wip label Apr 10, 2017
@mtho11
Copy link
Contributor Author

mtho11 commented Apr 10, 2017

Ignoring the codeclimate warning here. The similiar code warning is a false positive and 3 instances of 'Replace double quotes with Single Quotes' should be in a different PR like we discussed last week.

@mtho11
Copy link
Contributor Author

mtho11 commented Apr 10, 2017

@miq-bot assign @abonas

@abonas
Copy link
Member

abonas commented Apr 12, 2017

@abonas yes, I don't usually link to BZs, screenshots, etc until it's not WIP.

why? :)

@abonas
Copy link
Member

abonas commented Apr 12, 2017

@miq-bot assign @karelhala
@karelhala please review

@miq-bot miq-bot assigned karelhala and unassigned abonas Apr 12, 2017
@abonas
Copy link
Member

abonas commented Apr 19, 2017

@karelhala @mtho11 what's the status of this bug fix?

@mtho11
Copy link
Contributor Author

mtho11 commented Apr 19, 2017

@abonas from my perspective it is done. Just need @karelhala to verify the fix doesn't show repeated items in the existing JDBC driver list. So, the test is to add several JDBC drivers and see that they show up and do not repeat.

@karelhala
Copy link
Contributor

Github didn't submit my codereview...

Looks good, however as codeclimate complains use single quotes instead of double quotes please.

@miq-bot
Copy link
Member

miq-bot commented Apr 19, 2017

Checked commits https://github.com/mtho11/manageiq-ui-classic/compare/52e2c50e2044bcc238542025428b804eb50745f0~...1d9ca128dc54919271857ff5c67d924e02026e9d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks good. 🍪

@mtho11
Copy link
Contributor Author

mtho11 commented Apr 19, 2017

Double quotes changed to single quotes. The other codeclimate issue is a false positive.
@abonas good to go.

@abonas
Copy link
Member

abonas commented Apr 19, 2017

@miq-bot assign @dclarizio

@miq-bot miq-bot assigned dclarizio and unassigned karelhala Apr 19, 2017
@dclarizio dclarizio merged commit e928b35 into ManageIQ:master Apr 20, 2017
@dclarizio dclarizio added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 20, 2017
@mtho11 mtho11 deleted the ds_add_via_existing_driver2 branch April 20, 2017 18:30
simaishi pushed a commit that referenced this pull request Apr 20, 2017
Fix Add Datasource via existing driver Issue
(cherry picked from commit e928b35)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 754057083a489f5f52d5ec107542cb7b37e4489d
Author: Dan Clarizio <[email protected]>
Date:   Thu Apr 20 03:05:41 2017 -0700

    Merge pull request #941 from mtho11/ds_add_via_existing_driver2
    
    Fix Add Datasource via existing driver Issue
    (cherry picked from commit e928b351000817355397803a2a99db219d0cb6fc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants