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

Add blocklist to NameRes #709

Merged
merged 39 commits into from
Aug 15, 2023
Merged

Add blocklist to NameRes #709

merged 39 commits into from
Aug 15, 2023

Conversation

gaurav
Copy link
Collaborator

@gaurav gaurav commented Jul 20, 2023

We are implementing a Blocklist in Translator at https://github.com/NCATSTranslator/Blocklists, which will consist of a list of CURIEs that must be excluded from NameRes. This PR implements this by modifying the NameRes restore job in three ways:

  1. By creating an ephemeral storage volume (whose size is determined by the blocklist.storage config) to store the Blocklist.
  2. By adding support for a GitHub Personal Access Token (stored as a Kubernetes secret whose name is determined by the blocklist.github_personal_access_token_secret variable, and whose key is fixed as github_personal_access_token), which will be used to download the Blocklist file if it is in a private GitHub repository.
  3. By modifying the restore.sh script in to download the Blocklist file (as a text file containing a list of CURIEs, one on each line, as specified by the blocklist.url config), convert it into a string of double-quoted CURIEs, build a delete query for deleting all of these CURIEs, and then execute it.

Also includes:

  • The correction to the default NameRes version and data file from PR Fix NameRes version and source #723
  • An increase to the memory available to the id-id table, since we now have more IDs.

@gaurav gaurav changed the base branch from develop to update-data-for-name-lookup July 20, 2023 20:07
@gaurav gaurav marked this pull request as ready for review July 26, 2023 15:35
Base automatically changed from update-data-for-name-lookup to develop August 1, 2023 18:06
Copy link
Collaborator

@YaphetKG YaphetKG left a comment

Choose a reason for hiding this comment

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

If the github access token is needed by all the instances(ci test, and prod) after this and if its going to be the same across them, maybe we need to save it in values-populated.yaml ,
which is encrypted aswell , and intended to use common values accross envs

@gaurav
Copy link
Collaborator Author

gaurav commented Aug 15, 2023

If the github access token is needed by all the instances(ci test, and prod) after this and if its going to be the same across them, maybe we need to save it in values-populated.yaml , which is encrypted aswell , and intended to use common values accross envs

Thanks for the suggestion, @YaphetKG! I've updated the template to set up a Kubernetes Secret based on the value stored in the values-populated.yaml file as you suggest.

@gaurav gaurav requested a review from YaphetKG August 15, 2023 02:40
Copy link
Collaborator

@YaphetKG YaphetKG left a comment

Choose a reason for hiding this comment

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

LGTM!

@YaphetKG YaphetKG merged commit 43ae15d into develop Aug 15, 2023
@YaphetKG YaphetKG deleted the add-blocklist-to-nameres branch August 15, 2023 20:38
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.

None yet

2 participants