-
Notifications
You must be signed in to change notification settings - Fork 1
Add stream parameter to check if download is required or not #4
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
Conversation
de435d6 to
a0547a7
Compare
for more information, see https://pre-commit.ci
osm_data_client/client.py
Outdated
|
|
||
| async def get_osm_data(geometry: Union[Dict[str, Any], str], **kwargs) -> RawDataResult: | ||
| async def get_osm_data( | ||
| geometry: Union[Dict[str, Any], str], stream: Optional[bool] = None, **kwargs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I think adding the stream param is not necessary, as this is captured by **kwargs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To get it from **kwargs, we can do something like this:
config = RawDataClientConfig.default()
if (stream := kwargs.pop("stream", False)):
config.stream = stream
client = RawDataClient(config=config)
return await client.get_osm_data(geometry, **kwargs)|
I also replaced |
for more information, see https://pre-commit.ci
|
I refactored this to use @emirfabio would love if you could review this to see if it makes sense / if you prefer another approach. |
|
@Sujanadh I'm not sure if Emir is around to check this over, but what do you think? Is it working for you? |
|
looks good to me. {
"metadata": {
"task_id": "9aa3a4e0-7252-4ba6-89f9-8c730c2c2afd",
"format_ext": "geojson",
"timestamp": "",
"size_bytes": 63219,
"file_name": "fmtm_data_extract",
"download_url": "https://s3.dualstack.us-east-1.amazonaws.com/production-raw-data-api/default/fmtm_data_extract_geojson_uid_9aa3a4e0-7252-4ba6-89f9-8c730c2c2afd.geojson",
"is_zipped": false,
"bbox": null
},
"path": null,
"data": {
"download_url": "https://s3.dualstack.us-east-1.amazonaws.com/production-raw-data-api/default/fmtm_data_extract_geojson_uid_9aa3a4e0-7252-4ba6-89f9-8c730c2c2afd.geojson",
"file_name": "fmtm_data_extract",
"process_time": "a second",
"query_area": "0.04 Sq Km",
"binded_file_size": "0.06 MB",
"zip_file_size_bytes": 63219
},
"extracted": false,
"original_path": null,
"extracted_files": null
}There are repeated keys and values in both |
Agree, there is a bit of duplication! For a future PR 😁 |
Description
streamparameter in theRawDataClientConfigin order to avoid unnecessarily downloading data extracts on disc.