Allow users to provide copying of optional files without failing. #41
Conversation
…ests, so a softlink at src/ level of wxflow is a hack-around
aerorahul
requested review from
CoryMartin-NOAA,
DavidHuber-NOAA and
DavidNew-NOAA
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #41 +/- ##
===========================================
+ Coverage 50.12% 50.35% +0.23%
===========================================
Files 18 18
Lines 1652 1672 +20
Branches 339 343 +4
===========================================
+ Hits 828 842 +14
- Misses 765 771 +6
Partials 59 59 ☔ View full report in Codecov by Sentry. |
| @@ -0,0 +1,15 @@ | |||
| { | |||
| // Use IntelliSense to learn about possible attributes. | |||
There was a problem hiding this comment.
are these vscode files supposed to be committed?
There was a problem hiding this comment.
They should be. There is nothing here that is user specific.
| import os | ||
| import sys | ||
|
|
||
| sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '../src'))) |
There was a problem hiding this comment.
is this file needed?
There was a problem hiding this comment.
Yes. It tells the package where the src is in relation to the tests. Some packages do not keep code under src/app, but directly at app/
Another use for this file is to provide test fixtures that are/could be used across tests. Eg. if test_a.py and test_c.py needs something that is common, it can be a pytest fixture defined in conftest.py.
| @@ -35,15 +38,25 @@ def sync(self): | |||
| Method to execute bulk actions on files described in the configuration | |||
| """ | |||
| sync_factory = { | |||
| 'copy': self._copy_files, | |||
| 'copy': self.copy_req, | |||
| 'copy_req': self.copy_req, | |||
There was a problem hiding this comment.
copy and copy_req seems redundant since their behavior is the same. Why not just have copy?
There was a problem hiding this comment.
I kept the distinction for clarity. If it were my choice, I would deprecate copy and make it explicit copy_req so it becomes clear.
Currently, we need to keep copy since that will break existing use.
There was a problem hiding this comment.
In that case, I would suggest comments to indicate that copy is moving towards deprecation and shouldn't be used in the future.
There was a problem hiding this comment.
Sounds good. If that is the consensus from the team, we can make that happen.
I will make a comment that copy will be deprecated by XYZ, and users should use copy_req instead.
There was a problem hiding this comment.
done @DavidNew-NOAA
Co-authored-by: DavidNew-NOAA <134300700+DavidNew-NOAA@users.noreply.github.com>
Description
This PR extends the
FileHandlercopymethod to allow users to specify if files to be copied are eitherrequiredoroptional.This is done by introducing
copy_reqandcopy_opt. By defaultcopy == copy_req.The use is same as before; simply use
copy_optinstead ofcopyfor copying optional files. If a file is not found, a warning will be presented. Forcopyorcopy_req, the error will be fatal.This PR additionally allows users to checkout and develop
wxflowin a vscode workspace environment.Type of change
How Has This Been Tested?
Tests have been added to the extended functionality
Checklist