Skip to content

Conversation

@laurayco
Copy link
Collaborator

@laurayco laurayco commented Aug 11, 2025

Using the IOUtils assertInPath to reduce duplicate code.

@laurayco laurayco added the wip Work in progress, do not merge label Aug 11, 2025
@benbroadaway benbroadaway changed the title Use the shared IOUtils assertInPath method for resource lookups resource-task: Use the shared IOUtils assertInPath method for resource lookups Aug 11, 2025
@laurayco laurayco removed the wip Work in progress, do not merge label Aug 11, 2025
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
Copy link
Collaborator

@ibodrov ibodrov Aug 13, 2025

Choose a reason for hiding this comment

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

Why does it need commons-compress now? IIRC, it is only needed if you use zip* methods from concord-common.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really sure? When I referenced the IOUtils class without it there were "ClassNotFound" errors for the Zip* classes which get imported in IOUtils. We (Ben and I) tried getting around that by using the concord-common dependency, but that didn't seem to confer transient dependencies? If you have ideas on how to fix this they would be welcome.

Copy link
Collaborator

Choose a reason for hiding this comment

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

commons-compress is not needed by the resource task, but it is a transitive dependency of concord-common. Since concord-common is provided scope, its transitive deps aren't loaded for the unit tests. It could probably just be test scope instead of provided here.

@ibodrov ibodrov merged commit 444e836 into master Aug 15, 2025
4 checks passed
@ibodrov ibodrov deleted the directory-enforcement branch August 15, 2025 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants