Skip to content

Made additional Customer fields optional for compatibility with the sandbox environment#302

Merged
Graeme22 merged 3 commits intotastyware:masterfrom
maneum1:master
Feb 11, 2026
Merged

Made additional Customer fields optional for compatibility with the sandbox environment#302
Graeme22 merged 3 commits intotastyware:masterfrom
maneum1:master

Conversation

@maneum1
Copy link
Contributor

@maneum1 maneum1 commented Feb 10, 2026

Description

Related issue(s)

Fixes ...

Pre-merge checklist

  • Code formatted correctly (check with make lint)
  • Passing tests locally (check with make test, make sure you have TT_REFRESH, TT_SECRET, and TT_ACCOUNT environment variables set)
  • New tests added (if applicable)
  • Docs updated (if applicable)

Please note that, in order to pass the tests, you'll need to set up your Tastytrade credentials as repository secrets on your local fork. Read more at CONTRIBUTING.md.

Made some variables in the Customer class optional to be compatible with tastytrades sandbox environment.

@maneum1
Copy link
Contributor Author

maneum1 commented Feb 10, 2026

Not sure why the tests are failing here, they all passed a little while ago in my repository?

@Graeme22
Copy link
Member

Not sure why the tests are failing here, they all passed a little while ago in my repository?

See here: http://github.com/tastyware/tastytrade?tab=contributing-ov-file tests don't use my credentials for safety

@maneum1
Copy link
Contributor Author

maneum1 commented Feb 10, 2026

I did enter them in my repository, do you need them to be added to your repository?

@maneum1
Copy link
Contributor Author

maneum1 commented Feb 10, 2026

These are the instructions I got from the github system regarding the issue with accessing my secrets.
In a nutshell it informs that the secrets need to be on the upstream repository.

Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.
2 references

The job failed because all tests that require API interaction returned the error: {'error_code': 'invalid_grant', 'error_description': 'Invalid JWT'}. This indicates the JWT credentials used during testing are invalid or expired.

Solution:

Check that the secrets TT_REFRESH_SANDBOX, TT_SECRET_SANDBOX, TT_ACCOUNT, TT_REFRESH, and TT_SECRET are correctly configured in the repository settings and have not expired.
If using sandbox/test credentials, ensure they are still valid and have the required permissions.
If tokens need refreshing, generate new ones and update the repository secrets.
Optionally, enhance test setup code to fail fast and clearly if authentication fails.

Example credential refresh (in GitHub):

Go to repository Settings → Secrets and variables → Actions.
Re-add or update the above secrets with valid values.

After updating your credentials, re-run the workflow. This should resolve the “invalid_grant”/“Invalid JWT” errors.

In my repository maneum1/tastytrade I have set those secrets, but on the repository tastyware/tastytrade where the tests are run as part of the pull request it appears my secrets are not reachable

I understand the issue now. When you create a pull request from your fork (maneum1/tastytrade) to the upstream repository (tastyware/tastytrade), GitHub Actions runs the workflow on the upstream repository (tastyware/tastytrade), not your fork.

The Problem:

The secrets you set in your fork (maneum1/tastytrade) are not accessible to the workflow running in the upstream repository (tastyware/tastytrade)
For security reasons, GitHub does not pass fork secrets to the upstream repository's pull request workflows

Solutions:

For Maintainers of tastyware/tastytrade (Recommended):
    Add the required secrets (TT_REFRESH_SANDBOX, TT_SECRET_SANDBOX, TT_ACCOUNT, TT_REFRESH, TT_SECRET) to the upstream repository (tastyware/tastytrade) settings
    Go to: tastyware/tastytrade → Settings → Secrets and variables → Actions
    Add each secret with valid values

Conditional Test Execution (Alternative - if upstream maintainers can't add secrets):
    Modify the workflow to skip integration tests when secrets are unavailable
    Update .github/workflows/python-app.yml:

YAML

  • name: Test with pytest
    run: |
    uv run pytest --cov=tastytrade --cov-report=term-missing tests/ --cov-fail-under=95 -v
    env:
    TT_REFRESH_SANDBOX: ${{ secrets.TT_REFRESH_SANDBOX }}
    TT_SECRET_SANDBOX: ${{ secrets.TT_SECRET_SANDBOX }}
    TT_ACCOUNT: ${{ secrets.TT_ACCOUNT }}
    TT_REFRESH: ${{ secrets.TT_REFRESH }}
    TT_SECRET: ${{ secrets.TT_SECRET }}
    continue-on-error: true # Allow tests to fail gracefully if secrets are missing

    Contact the upstream maintainers to add the sandbox credentials so integration tests can run on all PRs.

The most secure and recommended approach is option 1 - have the upstream repository maintainers add the appropriate sandbox/test credentials as secrets.

@Graeme22 Graeme22 merged commit 3d9fddc into tastyware:master Feb 11, 2026
0 of 4 checks passed
@Graeme22
Copy link
Member

Thanks!

@maneum1
Copy link
Contributor Author

maneum1 commented Feb 11, 2026

Hi @Graeme22 , thanks for accepting my pull request, I'd still though like to understand why my tests did not go through. As you can see I'm not yet experienced with the github system, creating proper pull request, perform proper test, ... Any advise I'd really appreciate.

Following your instructions I learned already a lot about the testing system, Thanks

@Graeme22
Copy link
Member

Hi @Graeme22 , thanks for accepting my pull request, I'd still though like to understand why my tests did not go through. As you can see I'm not yet experienced with the github system, creating proper pull request, perform proper test, ... Any advise I'd really appreciate.

Following your instructions I learned already a lot about the testing system, Thanks

You actually did it correctly, tests are supposed to run in your fork with your credentials

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