fix: no warning for metas in executor #3219
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3219 +/- ##
==========================================
- Coverage 90.48% 90.47% -0.01%
==========================================
Files 148 148
Lines 10324 10324
==========================================
- Hits 9342 9341 -1
- Misses 982 983 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Latency summaryCurrent PR yields:
Breakdown
Backed by latency-tracking. Further commits will update this comment. |
|
Can I get some opinions here, if that makes sense. So the point is:
|
926a2c3 to
8265b9a
Compare
|
I am not sure what's the strategy. I think having the metas defined in the |
Following line of thought: The general target of I am not sure, if I 100% agree with this line of thought above, but users complained, that they did not got any warning, when specifying deprecated arguments. So this was my idea of solving it: Get more rigid with arg parsing. |
|
the test is incorrect, that YAML in the test is not a valid Flow anyway. so the warning is correct, there is nothing called I'm guessing you wanted to test against the below instead, which gives a valid Flow with one executor and executor has metas name as yaml = '''jtype: Flow
version: 1
executors:
- uses:
jtype: BaseExecutor
metas:
name: hello
'''
with Flow().load_config(yaml):
passwhich gives no warning on current master |
d8702d2 to
35cce1c
Compare
If you use
metasinside an Executor definition, there was a warning, that it is an unknown argument beforehands. Even thought it is not really used inside the argparse, this is the correct way in my opinion.