You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@phoenix.apache.org by yaise <gi...@git.apache.org> on 2014/03/11 08:14:13 UTC
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
GitHub user yaise opened a pull request:
https://github.com/apache/incubator-phoenix/pull/15
PHOENIX-831 First Connection to Phoenix blows up if it is tenant specific
https://issues.apache.org/jira/browse/PHOENIX-831
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yaise/incubator-phoenix master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-phoenix/pull/15.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #15
----
commit 9aab70d62bb719ed49c1c9add73cdcc62cc047c7
Author: Somesh <ss...@salesforce.com>
Date: 2014-03-11T07:11:40Z
PHOENIX-831 First Connection to Phoenix blows up if it is tenant specific
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10492965
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
Wow, multiple choice. I love it! :-) Let's do (1) and don't worry about the test code that does a "new Property(TEST_PROPERTIES)" as the static TEST_PROPERTIES will never change.
Thanks! Great bug!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10503565
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixDriverTest.java ---
@@ -0,0 +1,48 @@
+package org.apache.phoenix.jdbc;
+
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.junit.*;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+public class PhoenixDriverTest {
+
+ @Before
+ public void registerTestDriver() throws Exception {
+ DriverManager.registerDriver(new PhoenixTestDriver());
+ }
+
+ @After
+ public void deRegisterTestDriver() throws Exception {
+ DriverManager.deregisterDriver(new PhoenixTestDriver());
--- End diff --
this seems wrong. I'll figure out a way to get a handle to the driver that I registered in @before method. sorry about that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10464447
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
I had thought of a subclass of properties where I would just change the copy constructor behavior and use this where appropriate . I can try that or a util. It's more generic than this solution .
Although this solution nicely encapsulates tenant related changes to phoenixConnection where it's really used. In case we add more tenant related info that needs to stripped out ( or added ) in the future, this is the only point of change
Somesh Sasalatti
[sent from my mobile device]
> On Mar 11, 2014, at 0:32, James Taylor <no...@github.com> wrote:
>
> In phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java:
>
> > @@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
> > props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
> > return props;
> > }
> > -
> Nice find. I'd rather keep things simple, though. I'm not worried about creating a new Properties here. How about a util method that just instantiating a new, empty constructor Properties and iterating through the entrySet of the one passed through to just add the values? I'd rather do that then special case tenantId.
>
> —
> Reply to this email directly or view it on GitHub.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10492698
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
Will do. 2 options now :
1) I can replace it with something like PropUtils.deepCopy(Properties prop)
OR
2) new PropertiesCopy(props) where PropertiesCopy extends Property and I just provide a custom copy constructor to do the deep copy.
1 or 2 ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10508502
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixDriverTest.java ---
@@ -0,0 +1,48 @@
+package org.apache.phoenix.jdbc;
+
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.junit.*;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+public class PhoenixDriverTest {
+
+ @Before
+ public void registerTestDriver() throws Exception {
+ DriverManager.registerDriver(new PhoenixTestDriver());
+ }
+
+ @After
+ public void deRegisterTestDriver() throws Exception {
+ DriverManager.deregisterDriver(new PhoenixTestDriver());
+ }
+
+ @Test
+ public void testFirstConnectionWhenPropsHasTenantId() throws Exception {
+ final String url = "jdbc:phoenix:localhost";
--- End diff --
(see above)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10503636
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixDriverTest.java ---
@@ -0,0 +1,48 @@
+package org.apache.phoenix.jdbc;
+
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.junit.*;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+public class PhoenixDriverTest {
+
+ @Before
+ public void registerTestDriver() throws Exception {
+ DriverManager.registerDriver(new PhoenixTestDriver());
+ }
+
+ @After
+ public void deRegisterTestDriver() throws Exception {
+ DriverManager.deregisterDriver(new PhoenixTestDriver());
+ }
+
+ @Test
+ public void testFirstConnectionWhenPropsHasTenantId() throws Exception {
+ final String url = "jdbc:phoenix:localhost";
--- End diff --
connecting to jdbc:phoenix:localhost will blow up if hbase is not running right ? should I be using BaseTest here ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463718
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
+
+ /**
+ * Strips out tenant specific information from the passed in properties before creating a connection.
+ *
+ * @return a new Phoenix Connection.
+ * @throws SQLException
+ */
+ public static PhoenixConnection newTenantLessConnection(ConnectionQueryServices services, String url, Properties props, PMetaData metaData) throws SQLException {
+ if( props != null && props.getProperty(PhoenixRuntime.TENANT_ID_ATTRIB) != null ) {
+ //java.util.properties has a copy constructor but that doesn't do a deep copy.
+ // Instead it holds the props passed into the copy constructor as a reference(defaults) which is pretty much immutable.
+ // The tenantId might either be in a given property map or the property map it wraps.
+ if(props.containsKey(PhoenixRuntime.TENANT_ID_ATTRIB)) {
--- End diff --
I actually don't need another if/else block inside. The else block will work in both cases. just trying to avoid creating another copy if I don't have to, since PhoenixConnection's constructor creates a wrapper anyways.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463896
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
Nice find. I'd rather keep things simple, though. I'm not worried about creating a new Properties here. How about a util method that just instantiating a new, empty constructor Properties and iterating through the entrySet of the one passed through to just add the values? I'd rather do that then special case tenantId.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463695
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
Let me know if the comments here don't make sense. Basically the joy of using java.util.properties' copy constructor is that it doesn't do a deep copy but holds a reference. getProperty() works but native map operations like remove or containsKey only work on the map( i.e. this property instance) and not the one it wraps. So if someone added tenant ID to properties, great ( we can remove easily) but if someone added it to properties and then wrapped it in another one, then we have to create a new copy and exclude it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on the pull request:
https://github.com/apache/incubator-phoenix/pull/15#issuecomment-37269600
running mvn test now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463742
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixDriverTest.java ---
@@ -0,0 +1,48 @@
+package org.apache.phoenix.jdbc;
+
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.junit.*;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+public class PhoenixDriverTest {
+
+ @Before
+ public void registerTestDriver() throws Exception {
+ DriverManager.registerDriver(new PhoenixTestDriver());
+ }
+
+ @After
+ public void deRegisterTestDriver() throws Exception {
+ DriverManager.deregisterDriver(new PhoenixTestDriver());
+ }
+
+ @Test
+ public void testFirstConnectionWhenPropsHasTenantId() throws Exception {
+ final String url = "jdbc:phoenix:localhost";
+ verifyConnectionValid(url);
+ }
+
+ @Test
+ public void testFirstConnectionlessConnectionWhenPropsHasTenantId() throws Exception {
+ final String url = "jdbc:phoenix:none;test=true";
+ verifyConnectionValid(url);
+ }
+
+ private void verifyConnectionValid(String url) throws SQLException {
+ Driver driver = DriverManager.getDriver(url);
+
+ Properties props = new Properties();
+ final String tenantId = "00Dxx0000001234";
+ props.put(PhoenixRuntime.TENANT_ID_ATTRIB, tenantId);
+
+ Connection connection = driver.connect(url, props);
--- End diff --
This guy internally creates a *separate* metaConnection to do system table creation if it isn't already present. See Connection and ConnectionLessQueryImpl .
The connection that you are requesting should still contain tenantId though
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10481425
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
Yes, +1 on using a new util method in place of all "new Properties(Properties)" calls. I don't want to special case stuff for a certain property, as this will just come back to bite us again. Would you mind reworking your pull based on that and I'll commit it after that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463752
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriverTest.java ---
@@ -93,15 +93,15 @@ public void testNegativeGetConnectionInfo() throws SQLException {
"jdbc:phoenix:v1,v2,v3:123::/hbase",
"jdbc:phoenix:v1,v2,v3:123::/hbase;test=false",
};
- for (int i = 0; i < urls.length; i++) {
--- End diff --
just an intellij inspection refactor. ignore.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463772
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java ---
@@ -1251,7 +1251,7 @@ protected PhoenixConnection addColumnsIfNotExists(PhoenixConnection oldMetaConne
public void init(String url, Properties props) throws SQLException {
--- End diff --
This init method is exactly the same here and ConnectionlessQueryServicesImpl.java.
should be refactored, probably into static method somewhere as it doesn't affect the state of either classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463746
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriverTest.java ---
@@ -27,7 +27,7 @@
import org.apache.phoenix.exception.SQLExceptionCode;
import org.apache.phoenix.jdbc.PhoenixEmbeddedDriver.ConnectionInfo;
-public class PhoenixEmbeddedDriverTest {
+public class PhoenixEmbeddedDriverTest {
--- End diff --
!! will remove whitespace
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by JamesRTaylor <gi...@git.apache.org>.
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10508496
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixDriverTest.java ---
@@ -0,0 +1,48 @@
+package org.apache.phoenix.jdbc;
+
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.junit.*;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+public class PhoenixDriverTest {
+
+ @Before
+ public void registerTestDriver() throws Exception {
+ DriverManager.registerDriver(new PhoenixTestDriver());
+ }
+
+ @After
+ public void deRegisterTestDriver() throws Exception {
+ DriverManager.deregisterDriver(new PhoenixTestDriver());
--- End diff --
Derive from BaseConnectionlessQueryTest and use getUrl() if you need to get the connection url. This base class is for cases where we want to test client-side stuff, but not actually connect to an hbase cluster. Note that the DriverManager.deregisterDriver needs to use the singleton instance that was registered. You can get this through the static protected driver member variable. Why do you need to do this, though?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10503121
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
PhoenixConnection back to what it was. See PropertiesUtil.java
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10463666
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -41,15 +41,12 @@
import java.sql.Statement;
import java.sql.Struct;
import java.text.Format;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
+import java.util.*;
import java.util.concurrent.Executor;
import javax.annotation.Nullable;
+import com.google.common.base.Preconditions;
--- End diff --
unused import. I'll remove.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10501281
--- Diff: phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---
@@ -119,7 +116,36 @@ private static Properties newPropsWithSCN(long scn, Properties props) {
props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(scn));
return props;
}
-
--- End diff --
okay, util it is. Will only change the 2 places that we care to strip out the tenantId.
Thanks to Simon Toens who validated a "wierdness" I was seeing and gave me the code to test this. Our tests in core app were failing,
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-phoenix pull request: PHOENIX-831 First Connection to Ph...
Posted by yaise <gi...@git.apache.org>.
Github user yaise commented on a diff in the pull request:
https://github.com/apache/incubator-phoenix/pull/15#discussion_r10536763
--- Diff: phoenix-core/src/test/java/org/apache/phoenix/jdbc/PhoenixDriverTest.java ---
@@ -0,0 +1,48 @@
+package org.apache.phoenix.jdbc;
+
+import org.apache.phoenix.util.PhoenixRuntime;
+import org.junit.*;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+public class PhoenixDriverTest {
+
+ @Before
+ public void registerTestDriver() throws Exception {
+ DriverManager.registerDriver(new PhoenixTestDriver());
+ }
+
+ @After
+ public void deRegisterTestDriver() throws Exception {
+ DriverManager.deregisterDriver(new PhoenixTestDriver());
--- End diff --
Let me try that. My intention was to avoid any base classes that *might* do some pre work using a phoenix Connection. I wanted my test to be the first thing that connects to phoenix. Let me see what that base class does.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---