[HOPSWORKS-1972] Featurestore connect using API key value #76
Conversation
There was a problem hiding this comment.
Looks good, there is one copy paste error and some style issues that need some changes.
Also I think the test is flawed. First of all it is taking quite a while to fail if one doesn't change the host to an existing cluster, and if it is unable to do the rest call it will raise an exception and won't get to the assertion.
| @@ -53,6 +53,21 @@ | |||
| .willReturn(success().body(HttpBodyConverter.json(credentials))) | |||
| )); | |||
|
|
|||
| @Test | |||
| public void testReadAPIKey() throws IOException, FeatureStoreException { | |||
There was a problem hiding this comment.
This test somehow takes 2 minutes for me, I haven't changed the host etc.
There was a problem hiding this comment.
The options I see are:
- comment it out and leave the code there, so it can easily be tested again
- delete the code.
What do you think?
There was a problem hiding this comment.
We can leave it commented out, but I still believe the hc.getFeatureStore() will never return null but instead throw a exception if it can't get the feature store reference and hence the test will never get to the assert and always pass as the exception gets caught by the catch statement.
java/src/test/java/com/logicalclocks/hsfs/TestHopsworksExternalClient.java
Outdated
Show resolved
Hide resolved
java/src/main/java/com/logicalclocks/hsfs/metadata/HopsworksExternalClient.java
Show resolved
Hide resolved
|
@jimdowling I don't see any new commits, did you push them? |
https://logicalclocks.atlassian.net/browse/HOPSWORKS-1972
Python API Tested against:
https://colab.research.google.com/drive/1ZrOZzBLS4E0ue85sfRwISj5FlL2YiPfV?usp=sharing
Java API - new unit Test added in TestHopsworksExternalClient.java that needs an external cluster (but the unit test does not fail if external cluster not available).