Add support for getting artifacts #40
Conversation
msabramo
changed the title
| return self.splitext()[1] | ||
|
|
||
| def fnmatch(self, pattern): | ||
| return fnmatch.fnmatch(self.name, pattern) |
There was a problem hiding this comment.
Is this to be used when you are going through a collection of artifacts to see which one matches a glob pattern?
There was a problem hiding this comment.
If yes, what would be the scenario? (pseudo code)
There was a problem hiding this comment.
I'm basically wondering if we could have something like an ArtifactFinder class that would hide this detail and do:
af = ArtifactFinder(...)
artifacts = af.find("*.xml") # returns collection of artifacts... so you don't have to expose Artifact.fnmatch(), the artifact finder could just run fnmatch(Artifact.name, "*.xml").
There was a problem hiding this comment.
I don't have a use case in mind for Artifact.fnmatch(). I implemented it to have parity with path.py.
The ArtifactFinder is a good idea, though I think there are some methods here that do that (again, I borrowed them from path.py). Look at the dirs, files, and listdir methods.
There was a problem hiding this comment.
My recommendation is to not eagerly implement methods that you expose as a public API. If you haven't put much thought into and and you make it public, well then you are stuck with it. Every time I implement code I ask myself 2 things:
- do I have a use case for it?
- if yes and it's a public API, is it really mature enough to be implemented knowing that if I need to change the signature later then I won't be able to make backwards incompatible changes easily.
You'd have to write tests for it too, which is additional work. But granted, this is not necessarily the riskiest API but still worth considering.
msabramo
changed the title
The idea here is to have an
Artifactclass with an API that loosely resembles the API ofPathobjects in path.py, wherever it makes sense to.Cc: @aconrad