Skip to content

Conversation

@iamh2o
Copy link
Contributor

@iamh2o iamh2o commented Jan 30, 2026

Summary

This PR migrates Cognito configuration from .env to YAML config, adds background server mode to the CLI, and completes a full audit to eliminate all .env file usage from the codebase.

CI Fix

Fixed CI failures caused by -e ../daylily-tapdb editable install:

  • Checkout daylily-tapdb repository as sibling directory before conda setup
  • Update working directory for test steps
  • Remove python-dotenv from bloom_env.yaml

Configuration Audit (Complete)

Files Modified

File Changes
main.py Removed load_dotenv() import/call; fixed error message
bloom_lims/config.py Fixed template config path
bloom_lims/docs/cognito.md Complete rewrite to use YAML config
bloom_lims/docs/dewey.md Updated S3 prefix config section
README.md Updated config section + activation patterns
analytics/README.md Updated to use env vars + bloom_activate.sh
analytics/metabase/run_metabase.sh Removed .env sourcing
bloom_lims/bin/init_new_ubuntu22_ec2.sh Updated to YAML config + bloom_activate.sh
bloom_lims/bin/start_postgres_shell.sh Removed conda activate
bloom_lims/env/install_pgadmin.sh Changed to check $CONDA_DEFAULT_ENV
bloom_lims/env/install_postgres.sh Added clarifying comment
requirements.txt Removed python-dotenv~=1.0.0
bloom_env.yaml Removed python-dotenv==1.0.0
.github/workflows/*.yaml Checkout daylily-tapdb as sibling

Verification Results

Check Status
load_dotenv imports ✅ None found
.env file references ✅ None found
python-dotenv dependency ✅ Removed
Documentation updated ✅ All YAML config
Activation pattern ✅ Uses bloom_activate.sh
Tests ✅ 360 passed, 7 skipped

YAML Configuration

  • Add from_settings() class method to CognitoAuth for YAML config loading
  • Update get_cognito_auth() to use ~/.config/bloom/bloom-config.yaml (with env fallback)
  • Update get_allowed_domains() to use YAML config for email domain whitelist

Config file location: ~/.config/bloom/bloom-config.yaml

Config structure:

auth:
  cognito_user_pool_id: us-west-2_xxx
  cognito_client_id: xxx
  cognito_client_secret: xxx
  cognito_region: us-west-2
  cognito_domain: xxx.auth.us-west-2.amazoncognito.com
  cognito_redirect_uri: http://localhost:8911/oauth_callback
  cognito_allowed_domains:
    - lsmc.bio
    - lsmc.com

Environment variable overrides: Use BLOOM_ prefix with __ for nested keys:

export BLOOM_AUTH__COGNITO_REGION=us-east-1
export BLOOM_AUTH__COGNITO_ALLOWED_DOMAINS='["example.com"]'

CLI Improvements

  • Add background mode support to bloom gui command (--background/-b flag)
  • Add bloom gui stop command to stop background server
  • Add server logs command to view logs from ~/.bloom/logs/
  • Change default port from 8080 to 8911

Testing

Tested with shared Cognito pool:

  • Authentication with johnm+aug@lsmc.com succeeds
  • Background mode starts/stops correctly
  • All 360 tests pass

Related


Pull Request opened by Augment Code with guidance from the PR author

When using implicit OAuth flow, Cognito redirects to /oauth_callback#access_token=...
The URL fragment (after #) is not sent to the server in HTTP requests.
This GET handler serves an HTML page with JavaScript that:
1. Parses the URL fragment to extract tokens
2. POSTs the tokens to the existing POST handler
3. Handles errors and redirects appropriately
## YAML Configuration
- Add from_settings() class method to CognitoAuth for YAML config loading
- Update get_cognito_auth() to use ~/.config/bloom/bloom-config.yaml (with env fallback)
- Update get_allowed_domains() to use YAML config for email domain whitelist

## CLI Improvements
- Add background mode support to 'bloom gui' command (--background/-b flag)
- Add 'bloom gui stop' command to stop background server
- Add server logs command to view logs from ~/.bloom/logs/
- Change default port from 8080 to 8911

## Configuration Location
Config file: ~/.config/bloom/bloom-config.yaml
Logs: ~/.bloom/logs/
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b30a013e0f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Document shared Cognito pool configuration, YAML config format,
email domain whitelist, and server CLI commands.
- Remove python-dotenv dependency and load_dotenv() calls from main.py
- Fix template config path in bloom_lims/config.py
- Update cognito.md with complete YAML config documentation
- Update dewey.md S3 prefix config section
- Update README.md configuration and activation patterns
- Update analytics/README.md to use env vars and bloom_activate.sh
- Remove .env sourcing from analytics/metabase/run_metabase.sh
- Update init_new_ubuntu22_ec2.sh to use YAML config
- Update shell scripts to use bloom_activate.sh pattern
- Remove python-dotenv from requirements.txt

Configuration now uses:
1. Environment variables with BLOOM_* prefix
2. ~/.config/bloom/bloom-config.yaml
3. config/bloom-config-template.yaml defaults
- Modify ubuntu.yaml and macos.yaml workflows to checkout daylily-tapdb
  repository as a sibling directory before conda environment setup
- Update working-directory for test steps to use bloom/ subdirectory
- Remove python-dotenv from bloom_env.yaml (already removed from requirements.txt)

This fixes CI failures caused by '-e ../daylily-tapdb' editable install
in bloom_env.yaml requiring the sibling directory to exist.
@iamh2o iamh2o merged commit d0e391e into main Feb 1, 2026
4 checks passed
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.

1 participant