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

Support retrieving status and configuration of feed triggers #80

Merged
merged 2 commits into from Nov 9, 2017
Merged

Support retrieving status and configuration of feed triggers #80

merged 2 commits into from Nov 9, 2017

Conversation

abaruni
Copy link
Contributor

@abaruni abaruni commented Oct 25, 2017

This PR is to add support for retrieving the status and configuration of a feed trigger

Support has been implemented in the feed providers with the following:

apache/openwhisk-package-kafka#217 <-- Merged
apache/openwhisk-package-cloudant#137 <-- Merged
apache/openwhisk-package-alarms#101 <-- Merged

@jthomas
Copy link
Member

jthomas commented Oct 26, 2017

Code changes LGTM with the following additions...

### retrieve feeds

ow.feeds.get({feedName: '...', trigger: '...'})

The following optional parameters are supported:
...

@abaruni
Copy link
Contributor Author

abaruni commented Oct 31, 2017

@jthomas yes, this is a WIP. work on the integration tests is needed.

i've already modified the readme as follows

### retrieve resource

...
ow.feeds.get({name: '...'})

i think it best to match the pattern that is here. i.e., when the UPDATE lifecycleEvent is delivered, change the

### create & delete feeds

section to be

### create & update feeds

and remove the ow.feeds.delete({feedName: '...', trigger: '...'}) snippet in order to match

### create & update rule
### create & update packages
### create & update trigger

and then change the delete resource section to look like

### delete resource

...
ow.feeds.delete({name: '...'})

@codecov-io
Copy link

codecov-io commented Nov 6, 2017

Codecov Report

Merging #80 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
+ Coverage   99.41%   99.41%   +<.01%     
==========================================
  Files          14       14              
  Lines         339      340       +1     
==========================================
+ Hits          337      338       +1     
  Misses          2        2
Impacted Files Coverage Δ
lib/feeds.js 100% <100%> (ø) ⬆️

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 537b547...20dcac4. Read the comment docs.

@abaruni
Copy link
Contributor Author

abaruni commented Nov 7, 2017

@jthomas i've added to the integration test. mind having another look at this?

@abaruni
Copy link
Contributor Author

abaruni commented Nov 8, 2017

@csantanapr

@jthomas
Copy link
Member

jthomas commented Nov 9, 2017

@abaruni Code LGTM, still need the README.md changes? Your suggestions above about the doc structure make sense. 👍

@csantanapr csantanapr changed the title WIP Support retrieving status and configuration of feed triggers Support retrieving status and configuration of feed triggers Nov 9, 2017
@csantanapr csantanapr self-requested a review November 9, 2017 15:40
@csantanapr csantanapr self-assigned this Nov 9, 2017
Copy link
Member

@csantanapr csantanapr left a comment

Choose a reason for hiding this comment

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

LGTM

@csantanapr csantanapr merged commit 784df05 into apache:master Nov 9, 2017
@jthomas jthomas mentioned this pull request Nov 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants