Skip to content

Conversation

maxandersen
Copy link
Contributor

But this just doesn't seem right - shouldn't the client only do this if
sslChecks==true ?! Why both authkey/authiv and sslchecks booleans ?

…ng else specified.

Now prefixing with OpenShift- if another agent is specified.
@maxandersen
Copy link
Contributor Author

Not all test passes for me when building the client, but the tests I added for this does.

I do though not understand why we got a boolean "sslchecks" that is not used for anything - shouldn't that boolean be what is used to decide wether the "magic" OpenShift useragent is set ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added to allow actually testing the code.

@adietish
Copy link
Member

ssl certification checks and authkey are not related. The fact that the ssl-checks boolean is not implemented is a technical dept when creating the new REST client and trying to be on par with the legacy-client featurewise. The flag for the ssl checks should definitely get some logic/functionality.

@adietish
Copy link
Member

I dont see what tests dont pass for you. I had the following after merging your patch in:

Tests run: 140, Failures: 0, Errors: 0, Skipped: 17, Time elapsed: 10.791 sec

Results :

Tests run: 140, Failures: 0, Errors: 0, Skipped: 17

[INFO]
[INFO] --- maven-jar-plugin:2.4:jar (default-jar) @ openshift-java-client ---
[INFO] Building jar: /home/adietish/jboss-workspaces/openshift-java-client/target/openshift-java-client-2.0.2-> SNAPSHOT.jar
[INFO]
[INFO] --- maven-source-plugin:2.1.2:jar-no-fork (attach-sources) @ openshift-java-client ---
[INFO] Building jar: /home/adietish/jboss-workspaces/openshift-java-client/target/openshift-java-client-2.0.2-SNAPSHOT-sources.jar
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 18.165s
[INFO] Finished at: Wed Nov 21 10:47:33 CET 2012
[INFO] Final Memory: 17M/181M
[INFO] ------------------------------------------------------------------------

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

Successfully merging this pull request may close these issues.

2 participants