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

refactor: rock connection #764

Merged
merged 6 commits into from
Sep 10, 2024
Merged

refactor: rock connection #764

merged 6 commits into from
Sep 10, 2024

Conversation

marikaris
Copy link
Collaborator

@marikaris marikaris commented Jun 25, 2024

This is a refactor of the rock connection class, using the spring RestClient, rather than the (deprecated) RestTemplate, as suggested by spring. This should prevent blocking calls and overall work more smoothly.

Included apache in gradle, because default calls will go via jetty, which fails:
spring-projects/spring-boot#38960

@marikaris marikaris marked this pull request as draft June 25, 2024 14:10
@marikaris marikaris marked this pull request as ready for review July 4, 2024 08:42
Copy link

sonarqubecloud bot commented Jul 5, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@marikaris marikaris changed the title Refactor/rock connection refactor: rock connection Jul 5, 2024
@marikaris marikaris requested a review from mswertz July 5, 2024 10:44
@marikaris
Copy link
Collaborator Author

Codecov is failing, but it's a refactor of existing code. Can't find where it should have more tests.

}
};
private String getAuthHeader() {
String auth = application.getUser() + ":" + application.getPassword();
Copy link
Member

Choose a reason for hiding this comment

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

password?

Copy link
Member

@mswertz mswertz Sep 8, 2024

Choose a reason for hiding this comment

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

This is such that a request from rock can use the password to authenticate so we know if the proces (that armadillo started via rock) has access to the data (from that user)?
Kinda scary, is there not a toke we could use instead? Or a way to at least encrypt this? Or is the internal communication also SSL? Because untrusted R code might now abuse it? Or to go even further, we could simply use a session number to reconnect because the password doesn't add much security?

@marikaris marikaris merged commit d4a6fe4 into master Sep 10, 2024
2 of 3 checks passed
@marikaris marikaris deleted the refactor/rock-connection branch September 10, 2024 10:34
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