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.

Sign up for GitHub

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

handle long requests with POST #42

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

handle long requests with POST #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c2447bf
Properly handling long requests with POSTS.
ESapenaVentura Jan 19, 2023
e040dfb
Fixed last GET method
ESapenaVentura Jan 19, 2023
b4b5698
Fixed issue with empty accessions in query
ESapenaVentura Jan 19, 2023
800203c
Bumped to 1.0.23
ESapenaVentura Jan 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion geo_to_hca/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import os

version = '1.0.22'
version = '1.0.23'
from .config import Config

config = Config(os.environ)
42 changes: 28 additions & 14 deletions geo_to_hca/utils/entrez_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from xml.etree import ElementTree as xm

import requests
import requests as rq

from geo_to_hca import config
from geo_to_hca.utils import handle_errors
Expand Down Expand Up @@ -48,13 +47,17 @@ def call_esummary(accession, db='gds'):

def get_entrez_esearch(term, db="sra"):
throttle()
esearch_response = requests.get(url=f'{config.EUTILS_BASE_URL}/esearch.fcgi',
params={
"db": db,
"term": term,
"usehistory": "y",
"format": "json",
})
params={
"db": db,
"usehistory": "y",
"format": "json",
}
if len(term) <= 200:
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't use "magic numbers" in the code.
Define it as a constant.
If the number's source's external documentation, please add a link to the relevant document.

Copy link
Contributor Author

@ESapenaVentura ESapenaVentura Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The documentation is very cryptic, literally says more than several hundred characters long for eSearch, and if more than about 200 UIDs are to be provided, the request should be made using the HTTP POST method for EFetch. That's why I chose 200, the terms field can probably take more characters but I'd need to make tests to see how many hundred.

The documentation is here https://www.ncbi.nlm.nih.gov/books/NBK25499/#chapter4.ESearch for Esearch and https://www.ncbi.nlm.nih.gov/books/NBK25499/#_chapter4_EFetch_ for EFetch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add it as a constant, though!

params['term'] = term
esearch_response = requests.get(url=f'{config.EUTILS_BASE_URL}/esearch.fcgi', params=params)
else:
esearch_response = requests.post(url=f'{config.EUTILS_BASE_URL}/esearch.fcgi', params=params, data=f"term={term}")

log.debug(f'esearch url: {esearch_response.url}')
log.debug(f'esearch response status: {esearch_response.status_code}')
log.debug(f'esearch response content: {esearch_response.text}')
Expand Down Expand Up @@ -84,8 +87,8 @@ def call_efetch(db, accessions=[],
params= {
'db': db,
}
if accessions:
params['id'] = ",".join(accessions)

accessions_string = ",".join(accessions)
if webenv:
params['WebEnv'] = webenv
if query_key:
Expand All @@ -95,14 +98,25 @@ def call_efetch(db, accessions=[],
if retmode:
params['retmode'] = retmode
if mode == 'call':
efetch_response = rq.get(url, params=params)
if len(accessions_string) <= 200:
if accessions_string:
params['id'] = accessions_string
efetch_response = requests.get(url, params=params)
else:
efetch_response = requests.post(url, params=params, data=f"id={accessions_string}")
if efetch_response.status_code == STATUS_ERROR_CODE:
raise handle_errors.NotFoundSRA(efetch_response, accessions)
return efetch_response
elif mode == 'prepare':
return Request(method='GET',
if len(accessions_string) <= 200:
Copy link
Contributor

Choose a reason for hiding this comment

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

My earlier comment about the '200' magic number is amplified here.
Using something more then once is even a stringer reason to have it as a constant.

if accessions_string:
params['id'] = accessions_string
return Request(method='GET',
url=f'{config.EUTILS_BASE_URL}/efetch.fcgi',
params=params).prepare()
return Request(method='POST',
url=f'{config.EUTILS_BASE_URL}/efetch.fcgi',
params=params, data=f'id={accessions_string}').prepare()
else:
raise ValueError(f'unsupported call mode for efetch: {mode}')

Expand All @@ -112,7 +126,7 @@ def request_bioproject_metadata(bioproject_accession: str):
Function to request metadata at the project level given an SRA Bioproject accession.
"""
throttle()
srp_bioproject_url = rq.get(
srp_bioproject_url = requests.get(
f'{config.EUTILS_BASE_URL}/efetch/fcgi?db=bioproject&id={bioproject_accession}')
if srp_bioproject_url.status_code == STATUS_ERROR_CODE:
raise handle_errors.NotFoundSRA(srp_bioproject_url, bioproject_accession)
Expand All @@ -124,7 +138,7 @@ def request_pubmed_metadata(project_pubmed_id: str):
Function to request metadata at the publication level given a pubmed ID.
"""
throttle()
pubmed_url = rq.get(
pubmed_url = requests.get(
f'{config.EUTILS_BASE_URL}/efetch/fcgi?db=pubmed&id={project_pubmed_id}&rettype=xml')
if pubmed_url.status_code == STATUS_ERROR_CODE:
raise handle_errors.NotFoundSRA(pubmed_url, project_pubmed_id)
Expand Down