DM-50999: Replace getPackageDir with ResourcePath #253
Conversation
timj
force-pushed
the
tickets/DM-50999
branch
from
b508c3f to
1e3b402
Compare
kfindeisen
left a comment
kfindeisen
left a comment
There was a problem hiding this comment.
Some concerns about the behavior changes:
| files = ResourcePath.findFileResources([self.path], file_filter=r".*\.yaml$") | ||
| for file in files: | ||
| if "QuickTemplate" in file: | ||
| print("GRAPH BUILD FILE: ", file) |
There was a problem hiding this comment.
I assume this bare print shouldn't be here?
| [self.path.join("_ingredients", forceDirectory=True)], file_filter=r".*\.yaml$" | ||
| ) | ||
| # Do not want anything from the injection subdirectory. | ||
| if "injection/" not in f.path |
There was a problem hiding this comment.
I believe we do want to validate these pipelines, though the "WithFakes" filter on line 94/101 clearly needs to be updated.
There was a problem hiding this comment.
They weren't being validated before and I don't know how any of this works to know how to fix the validation. The change triggered here is that the test is finding things in subdirectories whereas before it was only looking in a single directory.
There was a problem hiding this comment.
Yes, I'd call it a bug in #223. I can ticket it, but please mark it as TODO instead making it sound as if SI pipelines are exempt from validation.
There was a problem hiding this comment.
Great. Will do. Can add the ticket number once you've filed it.
The pipeline readers can already read ResourcePath URIs.
timj
force-pushed
the
tickets/DM-50999
branch
from
1e3b402 to
7d4aee1
Compare
2 participants
The pipeline readers can already read ResourcePath URIs.