OCHA-DAP / hdx-python-api

Python API for interacting with the HDX Data Portal
http://data.humdata.org
MIT License
80 stars 16 forks source link

HDXDSYS-561 Add support for env var HDX_KEY_STAGE, HDX_KEY_DEV etc. #61

Closed mcarans closed 7 months ago

mcarans commented 7 months ago

Update pre-commit config

Fixes #62

github-actions[bot] commented 7 months ago

Test Results

134 tests  +1   134 :white_check_mark: +1   1m 24s :stopwatch: +3s   1 suites ±0     0 :zzz: ±0    1 files   ±0     0 :x: ±0 

Results for commit 26e5c7d2. ± Comparison against base commit d4d67627.

:recycle: This comment has been updated with latest results.

coveralls commented 7 months ago

Coverage Status

coverage: 97.531% (-0.02%) from 97.546% when pulling 26e5c7d2dedebd589b08b536859563eed90bacde on env_var_test into d4d67627d5037d854d730fded0fd21f659f359db on main.

IanHopkinson commented 7 months ago

I tried cloning the repo to propose a fix, however I get a number of test fails before I touch anything:

  1. TestCKAN.test_create_dataset fails because the environment variable GSHEET_AUTH is not defined (as an aside I can't see the environment variables set on this repo). I'm not sure that using a Google Sheet in this situation is necessary - a CSV test fixture(s) would be easier.
  2. TestConfiguration.test_env_vars fails because I have HDX_KEY_STAGE defined in my .hdx_configuration.yaml
  3. TestDatasetCore.test_create_in_hdx and TestDatasetCore.test_update_in_hdx both fail with when trying to do remove(file.name) with an error like: >
    remove(file.name)
    E       PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\ianho\\AppData\\Local\\Temp\\tmptbfxct85' 
  4. TestDatasetUpdateResourcesLogic.test_dataset_update_resources fails because the paths to the resources are hardcoded assuming UNIX-like paths.

I'm happy to fix these tests for running on Windows but if you don't think that's a priority then please let me know.

mcarans commented 7 months ago

@IanHopkinson I've sent you GSHEET_AUTH. I used a mixture of Google drive remote urls and filestore uploads to ensure that resources work for both cases.

TestConfiguration.test_env_vars fails because I have HDX_KEY_STAGE defined in my .hdx_configuration.yaml

I've made some changes and tested defining hdx_key_stage in .hdx_configuration.yml and the tests pass now.

TestDatasetCore.test_create_in_hdx and TestDatasetCore.test_update_in_hdx both fail with when trying to do remove(file.name) with an error like: >

remove(file.name) E PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\Users\ianho\AppData\Local\Temp\tmptbfxct85'

I have changed to rely on closing the file deleting the temporary file instead of explicitly deleting it. Hopefully that should fix this problem.

TestDatasetUpdateResourcesLogic.test_dataset_update_resources fails because the paths to the resources are hardcoded assuming UNIX-like paths.

I've changed the UNIX paths I found to use join instead.

If you see any other Windows test failures, please go ahead and fix.

I've made the logic clearer so that HDX_KEY_STAGE is always preferred to HDX_KEY whether in environment variable, keyword argument or configuration.