Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WX-735 Fix incorrect and/or nondeterministic filesystem ordering #6930

Merged
merged 9 commits into from
Oct 14, 2022
+28 −13
Merged

WX-735 Fix incorrect and/or nondeterministic filesystem ordering #6930

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cd80f9c
`SortedMap` shot in the dark
aednichols Sep 26, 2022
8f5d62e
Comment
aednichols Sep 26, 2022
bcfbc4d
Fix compiling
aednichols Oct 6, 2022
bd0b743
Merge remote-tracking branch 'origin/develop' into aen_wx_735
aednichols Oct 6, 2022
330d10f
Filesystem priority
aednichols Oct 11, 2022
261a4f5
Leave room for higher priority
aednichols Oct 11, 2022
217445d
Simplify sort
aednichols Oct 11, 2022
3608bfd
Implement constants suggestion
aednichols Oct 13, 2022
80500ac
Remove redundant comment
aednichols Oct 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Leave room for higher priority
@aednichols
aednichols committed Oct 11, 2022
commit 261a4f50b9beab95aaed5e6995d531831f843e1f
2 changes: 1 addition & 1 deletion filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilderFactory.scala
View file
Original file line number Diff line number Diff line change
@@ -34,5 +34,5 @@ final case class BlobPathBuilderFactory(globalConfig: Config, instanceConfig: Co
}
}

override def priority = 1
override def priority = 100
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there somewhere that we could add documentation that we think we'll maintain? Knowing what filesystems have a priority override from default would be nice if we ever have to change this so we don't have to chase through all the filesystems.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is for sure a weakness of this solution. I definitely have a desire to be able to look at one place and see the relative priorities. But at the same time, that would require Cromwell core to add dependencies on ALL of the subordinate file systems, which is clearly not ideal either. I am very open to brainstorming, and since it's relatively simple it is easy to change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Pushed a new commit implementing the constants suggestion from mob time.

}