Skip to content

Conversation

@joelvdavies
Copy link
Contributor

Description

Don't merge this, just testing feasibility of having a common repo between microservices (see #160).

Testing instructions

Add a set up instructions describing how the reviewer should test the code

  • Review code
  • Check Actions build
  • Review changes to test coverage

Agile board tracking

Links to #160

@joelvdavies joelvdavies self-assigned this May 14, 2025
@joelvdavies joelvdavies added enhancement New feature or request dependencies Pull requests that update a dependency file labels May 14, 2025
@joelvdavies
Copy link
Contributor Author

joelvdavies commented May 14, 2025

Notes

Installing from GitHub

  • Pulling directly from the git repo (e.g. ims-common@git+https://github.com/joelvdavies/common-repo-test#egg=main) requires git to be installed in the system/docker image (https://stackoverflow.com/questions/65061121/git-repository-within-docker-python-image)
  • Alternatively can use ims-common @ https://github.com/joelvdavies/common-repo-test/archive/refs/tags/v0.0.1.zip if there is a zip folder in the release which does not require git to be installed
  • The point at which the package is pulled and installed is when the docker image is built - it can't change by accident after this point
  • Installing with python3.12 -m pip install .[dev] was not enough to force the update (this was when using #egg=main) - apparently it either needs to be via version tag to force it, or use --force-reinstall

Other

  • For development outside of docker - pip install can be used to just install the dev version of the common package in the local venv to make changes immediate. Inside docker would be trickier. But might work with adding a COPY of the local folder and installing it from there in the run command. Dev cli script?
  • For scripts, the base code can be in the common repo, then all that would be needed in the sub package is the entry point with any specific repo config (The common code for creating a cli script can do the same)
  • Requirements.txt would still be needed in the sub repos to capture specific sub dependencies, so the common repo would need to be loose on them
  • A limitation of this base repo approach is the config e.g. for the database - this sort of thing would still need to be imported from within the actual microservice, although the base database config model could be imported from the common repo, logger_setup.py is another similar instance requiring config from a specific location

@joelvdavies joelvdavies force-pushed the test-common-repo-#160 branch from 2f53011 to 81411f6 Compare May 14, 2025 13:56
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 83.16%. Comparing base (6edeb61) to head (81411f6).

Files with missing lines Patch % Lines
object_storage_api/main.py 0.00% 2 Missing ⚠️

❌ Your patch status has failed because the patch coverage (66.66%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #161      +/-   ##
===========================================
+ Coverage    79.29%   83.16%   +3.87%     
===========================================
  Files           26       24       -2     
  Lines          768      713      -55     
===========================================
- Hits           609      593      -16     
+ Misses         159      120      -39     

☔ 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants