Skip to content

Conversation

@stuartcampbell
Copy link
Collaborator

@stuartcampbell stuartcampbell commented Aug 27, 2025

This is a collection of some cleanup tasks that I kept meaning to do.

  1. Removed storing keys in database for testing
  2. Added api_key and admin_api_key fixture so tests can access an api key to use in testing
  3. Code formatting applied
  4. Fixed some code linting errors

@stuartcampbell stuartcampbell requested a review from Copilot August 27, 2025 18:43

This comment was marked as outdated.

@stuartcampbell stuartcampbell marked this pull request as draft August 28, 2025 14:25
@stuartcampbell stuartcampbell requested a review from Copilot August 28, 2025 14:43
@stuartcampbell stuartcampbell self-assigned this Aug 28, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR performs maintenance tasks to clean up testing infrastructure and code formatting before a release. The primary focus is removing insecure storage of API keys in the database and improving the test setup for API authentication.

  • Removed storage of secret keys in the database for enhanced security
  • Added dedicated API key fixtures for test authentication
  • Applied code formatting and import organization improvements

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/nsls2api/tests/conftest.py Added api_key and admin_api_key fixtures, removed inline key generation from db fixture
src/nsls2api/tests/api/test_admin_api.py Updated tests to use the new admin_api_key fixture instead of database queries
src/nsls2api/services/sync_service.py Added docstrings and applied code formatting
src/nsls2api/services/proposal_service.py Reorganized imports alphabetically
src/nsls2api/services/facility_service.py Applied code formatting to function signature
src/nsls2api/services/bnlpeople_service.py Commented out debug logging statement
src/nsls2api/models/apikeys.py Removed secret_key field from ApiKey model
src/nsls2api/infrastructure/security.py Removed secret key storage, fixed typo in return value
src/nsls2api/api/v1/proposal_api.py Reorganized imports and removed unused import
scripts/migrate_satellite_locations.py Deleted migration script file

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@stuartcampbell stuartcampbell marked this pull request as ready for review August 28, 2025 14:45
@stuartcampbell
Copy link
Collaborator Author

Self merging as @HarikaBishai has given 👍

@stuartcampbell stuartcampbell merged commit 56e9437 into NSLS2:main Aug 28, 2025
3 checks passed
@stuartcampbell stuartcampbell deleted the maintenance branch August 28, 2025 17:00
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