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.

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

Update storm cmdargs 'type' value to allow any model type name #1923

Merged
merged 18 commits into from
Oct 23, 2020

Conversation

Cisphyx
Copy link
Contributor

@Cisphyx Cisphyx commented Oct 21, 2020

No description provided.

@codecov
Copy link

codecov bot commented Oct 21, 2020

Codecov Report

Merging #1923 into master will increase coverage by 3.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1923      +/-   ##
==========================================
+ Coverage   93.42%   96.45%   +3.02%     
==========================================
  Files         139      139              
  Lines       26508    26513       +5     
==========================================
+ Hits        24765    25573     +808     
+ Misses       1743      940     -803     
Flag Coverage Δ
#linux 96.45% <100.00%> (+3.02%) ⬆️
#linux_replay ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
synapse/lib/storm.py 98.31% <100.00%> (+0.24%) ⬆️
synapse/lib/hiveauth.py 95.96% <0.00%> (-0.77%) ⬇️
synapse/lib/view.py 95.29% <0.00%> (-0.75%) ⬇️
synapse/lib/trigger.py 93.91% <0.00%> (-0.44%) ⬇️
synapse/lib/layer.py 97.80% <0.00%> (-0.07%) ⬇️
synapse/datamodel.py 94.67% <0.00%> (+0.23%) ⬆️
synapse/cortex.py 95.19% <0.00%> (+0.54%) ⬆️
synapse/lib/cell.py 98.07% <0.00%> (+0.74%) ⬆️
synapse/lib/stormtypes.py 97.54% <0.00%> (+0.85%) ⬆️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed1f620...1a887c0. Read the comment docs.

synapse/lib/storm.py Outdated Show resolved Hide resolved
@Cisphyx Cisphyx changed the title Add time/ival as storm command arg types Update storm cmdargs 'type' value to allow any model type name Oct 22, 2020
@@ -1838,7 +1864,7 @@ class LimitCmd(Cmd):

def getArgParser(self):
pars = Cmd.getArgParser(self)
pars.add_argument('count', type=int, help='The maximum number of nodes to yield.')
pars.add_argument('count', type='int', help='The maximum number of nodes to yield.')
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a breaking change we need to be aware of

Copy link
Contributor

Choose a reason for hiding this comment

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

should only apply to python storm commands ( since pure storm commands had to be serializeable ) so i'd call it an internal change, but yea. def need to be aware.

@vEpiphyte vEpiphyte added this to the 2.10.0 milestone Oct 22, 2020
@Cisphyx Cisphyx merged commit 45bb730 into master Oct 23, 2020
@Cisphyx Cisphyx deleted the storm-argparse-time branch October 23, 2020 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants