search

Harvard-ATG / lts-iiif-ingest-service

A Python library to interact with the Harvard LTS (Library Technology Services) media ingest solution, which takes images and metadata and serves them via IIIF at scale. IIIFingest manages ingest credentials, uploads images to S3 for ingest, creates IIIF manifests, creates and signs asset ingest requests, and tracks the status of ingest jobs.
https://pypi.org/project/IIIFingest/
Other
2 stars 2 forks source link

Start using a version matrix for unit tests #155

Closed ColeDCrawford closed 1 year ago

ColeDCrawford commented 1 year ago

Closes #150

Maybe split the CI into two jobs - linting shouldn't matter based on Python version?

Do we want to move install to just relying on setup.cfg rather than requirements-dev.txt? E.g. have a dev section in setup.cfg and then just pip install -e ".[dev]".

ColeDCrawford commented 1 year ago

Failing due to zoneinfo, which is used in ingest.py and auth.py. zoneinfo was introduced to the Python standard library in 3.9. So we should either find a back compatible option, or update the package to only have support for 3.9+. Mapping Color is using Python 3.9 so I am fine with either solution. What is Media Manager on?

arthurian commented 1 year ago

@ColeDCrawford We're running python 3.7 on a bunch of apps (media manager particularly) until our next upgrade cycle, so we may want to explore using https://pypi.org/project/backports.zoneinfo/ to support python<3.9. We could use the fallback approach suggested in the docs:

try:
    import zoneinfo
except ImportError:
    from backports import zoneinfo

And regarding the idea of just relying on setup.cfg and using pip install -e ".[dev]", that sounds like a good way to go and eliminate the extra requirements file. I'm in favor of that.

ColeDCrawford commented 1 year ago

@arthurian how much pinning do we want to do in setup.cfg.install_requires and setup.cfg.dev? Right now we have no version specs for setup.cfg.install_requires but are pointing at very specific versions in requirements.txt and requirements-dev.txt.

ColeDCrawford commented 1 year ago

Can't seem to get the checks to show as passed on the PR, even though they are here: https://github.com/Harvard-ATG/lts-iiif-ingest-service/actions/runs/3488949032. This didn't run the split out linting Action either; I have the triggers for that set to PR and manual, so maybe it only runs at the top of PRs? I assumed that if you changed files it would rerun.

I think that both of these are probably some odd edge case with Github Actions, as I split them partway through this PR so maybe the triggers are off. Can revisit if we encounter issues with it once this is in main. @arthurian - let me know how this looks and if you're good with it we can merge. I'm able to get this to install with pip install -e ".[dev]" now and we moved to only using setup.cfg for dependencies. Happy to hear opinions from @Tanvez or @jaguillette in case there are reasons to keep requirements.txt.

ColeDCrawford commented 1 year ago

Not sure what this will do with dependabot

ColeDCrawford commented 1 year ago

Yes, it looks like setup.cfg will eventually be fully deprecated in favor of pyproject.toml as a more flexible / easier to parse format, but probably not for a couple years.

I've been looking at https://github.com/dependabot/dependabot-core/issues/2133 and https://github.com/orgs/community/discussions/6456. It looks like Dependabot got setup.cfg support last spring: https://github.com/dependabot/dependabot-core/pull/3423. Dependabot does not yet have just got pyproject.toml support - PR to add it: https://github.com/dependabot/dependabot-core/pull/5661.

Github dependency graph is separate from Dependabot and officially only supports requirements.txt and pipfile.lock (looks like some support for setup.py and pipfile?): https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/about-the-dependency-graph