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

Crypt4GH #35

Merged
merged 12 commits into from
Sep 13, 2018
Merged

Crypt4GH #35

merged 12 commits into from
Sep 13, 2018

Conversation

dtitov
Copy link
Contributor

@dtitov dtitov commented Sep 6, 2018

Crypt4GH support in Data Out solution + additional unit-tests to cover it.

Some endpoints are extended, but only with optional parameters (e.g. IV), so it should not affect an existing behavior.

@dtitov dtitov added enhancement New feature or request Interoperability labels Sep 6, 2018
@dtitov dtitov self-assigned this Sep 6, 2018
@@ -2,6 +2,6 @@

public enum Format {

PLAIN, AES, AES128, AES256, GPG_SYMMETRIC, GPG_ASYMMETRIC
PLAIN, CRYPT4GH
Copy link
Contributor

Choose a reason for hiding this comment

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

You should still include AES as target format; even if i is not used at present. GPG can be discarded.

@AlexanderSenf
Copy link
Contributor

I just had one comment. Otherwise it looks good to me. I will try and deploy/test this code later this week.

@dtitov
Copy link
Contributor Author

dtitov commented Sep 10, 2018

Thanks! I'll fix the enum tomorrow.

@dtitov
Copy link
Contributor Author

dtitov commented Sep 11, 2018

Done.

@AlexanderSenf
Copy link
Contributor

I can't actually compile it now (I created a fresh Ubuntu image for this test):
[INFO] Reactor Summary:
[INFO]
[INFO] EGA Data API ....................................... SUCCESS [ 0.005 s]
[INFO] EGA Data API File Database ......................... FAILURE [ 21.291 s]

I keep looking..

@AlexanderSenf
Copy link
Contributor

Ok.. 'sudo mvn package' works.. it is just my setting in the fresh VM then!

@@ -241,7 +261,8 @@
<configuration>
<imageName>${dockerRegistry}/res:${image.version}</imageName>
<baseImage>java:8</baseImage>
<entryPoint>["java", "${debug.config}${debug.res.port}", "-jar", "/${project.build.finalName}.jar"]
<entryPoint>["java", "${debug.config}${debug.res.port}", "-jar",
"/${project.build.finalName}.jar"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because of formatting, I think that the "/${project.build.finalName}.jar" in newline will not work properly in debug mode, It should be on the same line. Can you check if it work with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I missed that. You are right, it should be on the same line, I'll fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -54,7 +54,10 @@
import static eu.elixir.ega.ebi.shared.Constants.FILEDATABASE_SERVICE;
import static eu.elixir.ega.ebi.shared.Constants.RES_SERVICE;

@Profile("LocalEGA")
Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed this in the integration test now..


APPLICATION FAILED TO START


Description:

Field fileService in eu.elixir.ega.ebi.dataedge.rest.FileController required a single bean, but 2 were found:
- remoteFileServiceImpl: defined in URL [jar:file:/nfs/ega/public/srv/homes/ega-prod/ngs_3/test/ega-data-api-dataedge-0.0.1-SNAPSHOT.jar!/BOOT-INF/classes!/eu/elixir/ega/ebi/dataedge/service/internal/RemoteFileServiceImpl.class]
- localEGARemoteFileServiceImpl: defined in URL [jar:file:/nfs/ega/public/srv/homes/ega-prod/ngs_3/test/ega-data-api-dataedge-0.0.1-SNAPSHOT.jar!/BOOT-INF/classes!/eu/elixir/ega/ebi/dataedge/service/internal/LocalEGARemoteFileServiceImpl.class]

I think we need to keep profiles for the EBI deployment, as long as we have multiple beans implementing the same interface. Alternatively, we could make one @primary

@anandmohan777 anandmohan777 merged commit 867e5e0 into master Sep 13, 2018
@blankdots blankdots deleted the crypt4gh branch September 14, 2018 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Interoperability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants