BulkTag via Community APIs #354

miriam.baglioni · 2023-10-23T12:19:43+02:00

miriam.baglioni commented

2023-10-23 12:19:43 +02:00

This PR presents an implementation of the bulk tagging done exploiting the community apis instead of the IS.

The isLookupUrl parameter is no more needed.
One parameter is added: production of type boolean to switch between the API in production and in beta

This PR presents an implementation of the bulk tagging done exploiting the community apis instead of the IS. The isLookupUrl parameter is no more needed. One parameter is added: `production` of type boolean to switch between the API in production and in beta

miriam.baglioni added 15 commits 2023-10-23 12:19:44 +02:00

9fc8ebe98b refactoring

b64a5eb4a5 Merge branch 'master' of https://code-repo.d4science.org/D-Net/dnet-hadoop

d9506035e4 [ZenodoApi] gone back to okhttp3 to send the payload.

e4b27182d0 [master] refactoring

ef2dd7a980 resolved conflicts

8621377917 [UsageCount] fixed typo in attribute name for datasource table

9e8e39f78a -

599828ce35 Merge branch 'master' of https://code-repo.d4science.org/D-Net/dnet-hadoop

a9ede1e989 Merge branch 'master' of https://code-repo.d4science.org/D-Net/dnet-hadoop

69dac91659 adding the new code to use the API instead of the Information Service

18bfff8af3 adding test classes and modifying test for bulktag

34358afe75 modified resource file, workflow anf default-config. Add 3g of memory Overhead and specified the shuffle partition in the wf confiduration. Removed the multiple instantiation in the wf because of different implementation of the spark job

f206ff42d6 modified code to use the the API. Removing not needed parameters. Rewritten the code to exploit the parallel stream on the entity types

70b78a40c7 removed file from different propagation

5c5a195e97 refactoring and fixing issue on property name

claudio.atzori was assigned by miriam.baglioni

2023-10-23 12:20:22 +02:00

claudio.atzori requested changes 2023-10-25 15:42:40 +02:00

claudio.atzori left a comment

I assume the procedure was tested by running the oozie workflow manually and verifying its output, so I'm not going to comment on that part but, although I understand the difficulties in automating the tests, we should try to do better in implementing the unit testing. I left some comments on this regard, so please let me know to what extent they can be addressed.

dhp-common/src/test/java/eu/dnetlib/dhp/common/api/ZenodoAPIClientTest.java Outdated

						
				@ -16,3 +16,3 @@

					private final String URL_STRING = "https://sandbox.zenodo.org/api/deposit/depositions";

					private final String ACCESS_TOKEN = "";

					private final String ACCESS_TOKEN = "OzzOsyucEIHxCEfhlpsMo3myEiwpCza3trCRL7ddfGTAK9xXkIP2MbXd6Vg4";

claudio.atzori commented

2023-10-25 15:09:42 +02:00

I'm not sure it is a good idea to hardcode the access token in the project source files.

miriam.baglioni commented

2023-10-26 09:38:43 +02:00

It is of course not :)

dhp-common/src/test/java/eu/dnetlib/dhp/common/api/ZenodoAPIClientTest.java Outdated

						
				@ -109,0 +113,4 @@

						Assertions.assertEquals(201, client.newDeposition());

						File file = new File("/Users/miriam.baglioni/Desktop/EOSC_DUMP/publication.tar");

claudio.atzori commented

2023-10-25 15:40:05 +02:00

avoid referring to your local filesystem

dhp-workflows/dhp-enrichment/src/main/java/eu/dnetlib/dhp/api/QueryCommunityAPI.java

						
				@ -0,0 +13,4 @@

				 * @author miriam.baglioni

				 * @Date 06/10/23

				 */

				public class QueryCommunityAPI {

claudio.atzori commented

2023-10-25 14:51:14 +02:00

Unit tests should not interact with remote endpoints (see point 7 here). To test the new API consider to exploit the annotation

org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest

used in the CommunityApiControllerTest.

Unit tests should not interact with remote endpoints (see point 7 [here](https://brightsec.com/blog/unit-testing-best-practices/)). To test the new API consider to exploit the annotation ``` org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest ``` used in the [CommunityApiControllerTest](https://code-repo.d4science.org/D-Net/dnet-applications/src/branch/master/apps/dnet-exporter-api/src/test/java/eu/dnetlib/openaire/community/CommunityApiControllerTest.java).

miriam.baglioni commented

2023-10-26 09:39:19 +02:00

Thanks for the suggestion. I will implement the testing following that trace

miriam.baglioni commented

2023-10-26 09:30:37 +02:00

Yes, I have run it on the cluster and verified only that the numbers are "more or less" the same. This is because with the API the configuration can change from a run to another, while what we have done and is on the graph is frozen at a given moment in time

The testing was done in a hurry, just to verify some functionalities. It was not intended to be left like that. I should have removed it before issuing the PR.

I have to remove from common the code related to the Zenodo APIs because I have model it on the dump project (It is used there alone)

Yes, I have run it on the cluster and verified only that the numbers are "more or less" the same. This is because with the API the configuration can change from a run to another, while what we have done and is on the graph is frozen at a given moment in time The testing was done in a hurry, just to verify some functionalities. It was not intended to be left like that. I should have removed it before issuing the PR. I have to remove from common the code related to the Zenodo APIs because I have model it on the dump project (It is used there alone)

miriam.baglioni added 1 commit 2023-10-26 09:38:22 +02:00

0097f4e64b Removed Query community testing. Removed package from common related to the interaction with Zenodo since it was moved to the dump-project

claudio.atzori added this to the OpenAIRE project 2023-10-26 09:40:44 +02:00

claudio.atzori modified the project from OpenAIRE to OpenAIRE - DNet

2023-10-26 09:58:28 +02:00

claudio.atzori manually merged commit 5f1ed61c1f into master

2023-11-03 12:52:15 +01:00

claudio.atzori commented

2023-11-03 15:51:03 +01:00

This PR presents an implementation of the bulk tagging done exploiting the community apis instead of the IS.

The isLookupUrl parameter is no more needed.
One parameter is added: production of type boolean to switch between the API in production and in beta

I realised now that base URL to the new API implementations available in the beta and prod environments are hardcoded and the production parameter allows to select among the two. Please consider to factor it out in a subsequent PR: having the possibility to define the API base URL fits well with the definition of a test case where the API server application is mocked locally.

> This PR presents an implementation of the bulk tagging done exploiting the community apis instead of the IS. > > The isLookupUrl parameter is no more needed. > One parameter is added: `production` of type boolean to switch between the API in production and in beta I realised now that base URL to the new API implementations available in the beta and prod environments are hardcoded and the `production` parameter allows to select among the two. Please consider to factor it out in a subsequent PR: having the possibility to define the API base URL fits well with the definition of a test case where the API server application is mocked locally.

Sign in to join this conversation.

No reviewers