Maintenance week updates

[jonavellecuerdo]

Purpose and background context

Update apps according to DataEng Application Maintenance document

Learnings/updates worth noting:

1. Set permission on cli.py as not executable: During initial linting with Ruff, the error code EXE002 was thrown for the harvester.cli. After some investigation (with @ghukill), it turned out that harvester.cli was set as an executable (i.e., chmod +x cli.py was run at some point). Rather than having Ruff ignore error EXE002, I decided to update the permission so that it is no longer executable. Why:

   • After updating the permission, I ran the steps outlined in README | Development and successfully built the Docker container image and ran the harvester from a container. This suggests that the executable permission is not required for our use case (or normal/expected functionality).

2. Why does the repo have setup.py: Discussed this with @ghukill as well! There are some repos that continue to have and use setup.py, have but NOT use setup.py, or don't have setup.py at all. For this repo, setup.py is used in the second stage of the Dockerfile (lines 4-7) to build the oai-pmh-harvester as a package, which allows us to directly call oai in the ENTRYPOINT of the Docker container. Decided to leave it as-is for now but would be nice to have a team discussion at some point to figure out how/whether we should standardize this!

3. Encapsulate configuration functions in a class
   

4. Update README: I referenced geo-harvester/README while making updates!

How can a reviewer manually see the effects of these changes?

1. Run make lint to verify repo passes all linters.
2. Run make test to verify all unit tests pass.

Includes new or updated dependencies?

YES

Changes expectations for external applications?

NO

Developer

• All new ENV is documented in README
• Stakeholder approval has been confirmed (or is not needed)

Code Reviewer(s)

• The commit message is clear and follows our guidelines (not just this PR message)
• There are appropriate tests covering any new functionality
• The provided documentation is sufficient for understanding any new functionality introduced
• Any manual tests have been performed or provided examples verified
• New dependencies are appropriate or there were no changes

[github-actions]

Pull Request Test Coverage Report for Build 7479069181

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 6931508858:	0.0%
Covered Lines:	201
Relevant Lines:	201

---

💛 - Coveralls

[ghukill]

I think it looks great! Thanks for this work.

I appreciate that you took the time to investigate the Config class approach. And, I think it probably would make sense for newer projects (like GeoHarvester) to emulate this approach of putting configure_logger() and configure_sentry() as methods on the class.

Also, README looks much better, with lots of helpful information.

I also agree about leaving setup.py as-is for now, and having a team discussion. As we discussed, definitely pros and cons to having the application installed "globally" in the docker container, or invoked via a virtual environment. I think maybe this was one of the earlier CLI apps, so maybe that explains it's difference here? Either way, even if they aren't all identical, a discussion would be nice.

All in all, looks like a nice freshen up, and able to run locally and via the StepFunction in Dev1 successfully.

[ehanson8]

Looks great, excellent improvements to the documentation!