-
Notifications
You must be signed in to change notification settings - Fork 0
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
EPA-55: Remove IP remapping when contacting RISE in the diga-epa-lib #6
Conversation
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.
LGTM, minor changes on my side
@@ -37,14 +36,10 @@ | |||
import org.junit.jupiter.api.BeforeEach; | |||
import org.junit.jupiter.api.Disabled; | |||
import org.junit.jupiter.api.Test; | |||
import org.slf4j.Logger; | |||
import org.slf4j.LoggerFactory; | |||
|
|||
@Disabled | |||
class KonnektorServiceTest { |
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.
class KonnektorServiceTest { | |
// e2e test to write sample document to our test environment | |
class KonnektorServiceAcceptanceTest { |
I think it would be helpful for future readers, also to explain why the test is disabled by default. feel free to rephrase better ofc
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.
renamed the class and added a comment
@@ -98,12 +93,13 @@ private KonnektorService buildService() throws Exception { | |||
|
|||
// these are the TLS client credentials as received from the Konnektor provider (e.g. RISE) | |||
var keys = loadKeys(); | |||
var uri = URI.create("https://localhost:4443"); | |||
var uri = URI.create("https://10.156.145.103:443"); |
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.
could you extract it into a constant? the other magic values as well, makes it easier to change it in the future if we need it
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.
done, good point
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.
🚀 🚀
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.
👌
No description provided.