Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Python3.11 to tests, README detail and bump dependencies #87

Merged
merged 11 commits into from
Feb 14, 2024

Conversation

patrick-austin
Copy link
Contributor

Changes

  • Add detail on devel packages to README. Got the list from the Ansible repo, but since that's private (and possible to overlook) thought it was worth including a public facing reference.
  • Update README and nox to target up to 3.11, and noted (at least one) reason why we can't target 3.12
  • Included 3.10 and 3.11 in the CI matrix
  • Bumped motor and orjson

Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (878c440) 83.18% compared to head (694ed48) 83.18%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #87   +/-   ##
=======================================
  Coverage   83.18%   83.18%           
=======================================
  Files          45       45           
  Lines        2153     2153           
  Branches      164      164           
=======================================
  Hits         1791     1791           
  Misses        329      329           
  Partials       33       33           

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

@MRichards99 MRichards99 self-requested a review February 12, 2024 10:07
Copy link
Collaborator

@MRichards99 MRichards99 left a comment

Choose a reason for hiding this comment

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

I just wanted to add a comment to remind me what I've seen when I come back to give a final review. Changes so far look good, just needs 3.10 & 3.11 CI sorting and the merge conflicts resolving

Copy link
Collaborator

@MRichards99 MRichards99 left a comment

Choose a reason for hiding this comment

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

To fix the issue with calling login() multiple places, just increase the number of minutes the access token is valid for. For GitHub Actions, you can do that at
https://github.com/ral-facilities/operationsgateway-api/blob/DSEGOG-287-Issues-setting-up-dev-environment/.github/ci_config.yml#L26

Looks good otherwise :)

@patrick-austin
Copy link
Contributor Author

To fix the issue with calling login() multiple places, just increase the number of minutes the access token is valid for. For GitHub Actions, you can do that at https://github.com/ral-facilities/operationsgateway-api/blob/DSEGOG-287-Issues-setting-up-dev-environment/.github/ci_config.yml#L26

Looks good otherwise :)

Thanks - nice that this is configurable. I went looking and saw:


And (incorrectly) jumped to the conclusion the lifetime of the token was hardcoded so would have to just spam login or refresh requests. Though thinking about it for any significant amount of time should've seen that 604800 is a week in seconds so can't have been anything to do with a token expiring after an hour...

@patrick-austin patrick-austin merged commit eb7f545 into main Feb 14, 2024
9 checks passed
@patrick-austin patrick-austin deleted the DSEGOG-287-Issues-setting-up-dev-environment branch February 14, 2024 16:17
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