Skip to content

Conversation

@rohansen856
Copy link

Metadata

Details

this PR is made to remove the hardcoded test server admin key from the codebase and replace it with environment variable-based authentication.

Summary

  • in openml/config.py Added a new environment variable constant for the test server admin key:
OPENML_TEST_SERVER_ADMIN_KEY_ENV_VAR = "OPENML_TEST_SERVER_ADMIN_KEY"
  • Testing Base Class Updated in openml/testing.py. Modified the TestBase class to read the admin key from an environment variable instead of using a hardcoded value:

Before:

admin_key = "abc"

After:

admin_key = os.environ.get(openml.config.OPENML_TEST_SERVER_ADMIN_KEY_ENV_VAR)

Note:

  • The admin key is now None by default when the environment variable is not set
  • Tests requiring the admin key will fail gracefully if the key is not available

Also in the tests, Added pytest.skipif decorators to tests that require admin privileges in the following test files:

tests/test_openml/test_config.py

Test: test_switch_to_example_configuration

Added decorator:

@pytest.mark.skipif(
    not os.environ.get(openml.config.OPENML_TEST_SERVER_ADMIN_KEY_ENV_VAR),
    reason="Test requires admin key. Set OPENML_TEST_SERVER_ADMIN_KEY environment variable.",
)

tests/test_datasets/test_dataset_functions.py

Test: test_data_status

Added decorator:

@pytest.mark.skipif(
    not os.environ.get(openml.config.OPENML_TEST_SERVER_ADMIN_KEY_ENV_VAR),
    reason="Test requires admin key. Set OPENML_TEST_SERVER_ADMIN_KEY environment variable.",
)

Note:

  • These tests will be automatically skipped if the admin key is not provided
  • Clear skip reason is displayed when tests are skipped
  • No failures or errors when running tests locally without the admin key

4. CI Configuration Update (.github/workflows/test.yml)

Added the environment variable to all test execution steps in the GitHub Actions workflow:

Steps updated:

  • Run tests on Ubuntu Test
  • Run tests on Ubuntu Production
  • Run tests on Windows

Added to each step:

env:
  OPENML_TEST_SERVER_ADMIN_KEY: ${{ secrets.OPENML_TEST_SERVER_ADMIN_KEY }}

Impact:

  • CI will use the secret stored in GitHub repository settings
  • Tests requiring admin privileges will run in CI
  • The actual key value is never exposed in logs or code

@PGijsbers this requires someone to put the admin key in the github secrets which would be a critical step.

Signed-off-by: rohansen856 <rohansen856@gmail.com>
Signed-off-by: rohansen856 <rohansen856@gmail.com>
Signed-off-by: rohansen856 <rohansen856@gmail.com>
Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

This successfully removes the secret from the code base.

My only question, is there a documentation location (e.g., developer documentation) that also needs to get updated about this? Since developers wishing to use the test server now have to:

  • obtain credentials (from whom or where?)
  • set the environment variable in python

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.38%. Comparing base (3380bbb) to head (7a94293).
⚠️ Report is 154 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (3380bbb) and HEAD (7a94293). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (3380bbb) HEAD (7a94293)
2 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1568       +/-   ##
===========================================
- Coverage   85.24%   64.38%   -20.86%     
===========================================
  Files          38       36        -2     
  Lines        5008     4352      -656     
===========================================
- Hits         4269     2802     -1467     
- Misses        739     1550      +811     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rohansen856
Copy link
Author

This successfully removes the secret from the code base.

My only question, is there a documentation location (e.g., developer documentation) that also needs to get updated about this? Since developers wishing to use the test server now have to:

  • obtain credentials (from whom or where?)
  • set the environment variable in python

I think if any documentation update has to be done it should be in Advanced user guide (https://openml.github.io/openml-python/latest/details/). As for local testing, there is no .env file in the codebase so it needs to be provided by the developer before testing (ex.: export OPENML_TEST_SERVER_ADMIN_KEY="your-admin-key" && pytest tests/) But even if a new developer does not have the context about this, The tests would automatically skip if the developer does not provide the variable so tests wont fail because of this.

also this is the admin API key for the OpenML test server (https://test.openml.org`) (even though we used very weak credentials for testing purposes: abc). so it should be behind a env var. So instead of having it exposed, using it as a secret in github actions would be better and i can update the documentation for any developer who want to actually run the two tests that would be skipped any way in case of local testing if they dont provide the value.

Please do let me know if you have other suggestions on this @fkiraly

@fkiraly
Copy link
Collaborator

fkiraly commented Dec 29, 2025

@jgyasu, does this need to be added to the developer guide you are writing?

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.

3 participants