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

Derek furst/sync component datasets #646

Merged
merged 8 commits into from
Apr 3, 2024

Conversation

DerekFurstPitt
Copy link
Contributor

Changed the status property in provenance schema yaml. Now, its after update trigger method is update_status, which calls set status history, and then calls a new function sync_component_dataset_status. this works by creating new PUT calls to entity on each of the component datasets, updating their status to whatever the multi-assay dataset was. A new validator was also created that disallows directly updating the status of component datasets.

@DerekFurstPitt DerekFurstPitt requested a review from yuanzhou March 22, 2024 18:06
def update_entity(uuid, data, token):
url = _entity_api_url + SchemaConstants.ENTITY_API_UPDATE_ENDPOINT + '/' + uuid
header = _create_request_headers(token)
header[schemaConstants.HUBMAP_APP_HEADER] = INGEST_API_APP
Copy link
Member

Choose a reason for hiding this comment

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

I didn't test, at a quick glance INGEST_API_APP should be referenced using schemaConstants..

token : string
The globus groups token
"""
def update_entity(uuid, data, token):
Copy link
Member

@yuanzhou yuanzhou Mar 22, 2024

Choose a reason for hiding this comment

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

Instead of defining this update_status() update_entity() in schema_manager, we should handle the business logic in the trigger method. Because the schema_manager should not care about updating the entity specifically.

We should add a getter method similar to

"""
Get the ingest-api URL to be used by trigger methods
Returns
-------
str
The ingest-api URL
"""
def get_ingest_api_url():
global _ingest_api_url
return _ingest_api_url

so other modules can get a handle of the entity-api url and then make request inside the trigger method like this
response = requests.post(url = ingest_api_target_url, headers = schema_manager._create_request_headers(user_token), json = json_to_post, verify = False)

@@ -32,6 +32,9 @@ INGEST_API_URL = 'https://ingest-api.dev.hubmapconsortium.org'
# Works regardless of the trailing slash
ONTOLOGY_API_URL = 'https://ontology-api.dev.hubmapconsortium.org'

# URL for talking to Entity API (default for DEV)
ENTITY_API_URL = 'https://entity-api.dev.hubmapconsortium.org'
Copy link
Member

Choose a reason for hiding this comment

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

Even though this is instructional, we should use localhost URL rather than DEV. Add some comment to explain this is the same URL base where the entity-api is running. Because if someone uses the DEV URL and runs the entity-api locally, it'll end up updating the component datasets in DEV neo4j from the local trigger requests. The key is to keep it consistent. And once deployed on DEV/TEST/PROD, we can still use a localhost:port where the entity-api runs on the VM.

sync_component_dataset_status rather than schema manager. created getter
get_entity_api_url
@DerekFurstPitt
Copy link
Contributor Author

@yuanzhou I applied the requested fixes. Take a look when you get a chance

@yuanzhou yuanzhou merged commit e36a1ab into dev-integrate Apr 3, 2024
3 checks passed
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