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

Added code for NewRelic V2 #876

Closed
wants to merge 8 commits into from
Closed

Added code for NewRelic V2 #876

wants to merge 8 commits into from

Conversation

116davinder
Copy link

@116davinder 116davinder commented Sep 9, 2020

SUMMARY

Added support for NewRelic Deployment V2 Api

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

newrelic_deployment

ADDITIONAL INFORMATION
ansible 2.5.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/davinderp/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Nov  6 2016, 00:28:07) [GCC 4.8.5 20150623 (Red Hat 4.8.5-11)]

After checking NewRelic Website, as they have depricated v1 api with xml format and they also exposed new v2 api with json/xml so i start this PR to support new v2 api and remove old version.

Original PR on Main Ansible Repoistory: ansible/ansible#40029

If this PR doesn't get merged than anyone can download same code from my personal repository: https://github.com/116davinder/ansible-custom-libs/blob/master/newrelic_deployment.py

@116davinder
Copy link
Author

Original PR on Main Ansible Repoistory: ansible/ansible#40029

@felixfontein
Copy link
Collaborator

Woudln't it make more sense to adjust the existing module so that it also can work with the v2 API?

It's not clear to me though (I don't know NewRelic at all) whether the v1 API is still available anywhere. If it is (and especially if the v2 API is not available everywhere), the module needs to support both APIs.

@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/newrelic_deployment_v2.py:152:0: missing-final-newline: Final newline missing

The test ansible-test sanity --test validate-modules [explain] failed with 7 errors:

plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'app_name' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'application_id' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'changelog' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'description' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'revision' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'token' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'user' in argument_spec uses default type ('str') but documentation doesn't define type

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/newrelic_deployment_v2.py:152:11: W292: no newline at end of file

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/newrelic_deployment_v2.py:152:0: missing-final-newline: Final newline missing

The test ansible-test sanity --test validate-modules [explain] failed with 9 errors:

plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'app_name' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'application_id' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'changelog' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'description' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'revision' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'token' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'user' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-required-mismatch: Argument 'revision' in argument_spec is required, but is not documented as being required
plugins/modules/newrelic_deployment_v2.py:0:0: module-invalid-version-added: DOCUMENTATION: version_added ('0.1') is not a valid collection version (see specification at https://semver.org/): invalid semantic version '0.1'. Got {'module': 'newrelic_deployment_v2', 'version_added': '0.1', 'author': 'Davinder Pal (@116davinder)', 'short_description': 'Notify newrelic about app deployments using newrelic v2 api', 'description': ['Notify newrelic about app deployments (https://docs.newrelic.com/docs/apm/new-relic-apm/maintenance/record-deployments)'], 'options': {'token': {'description': ['API token, to place in the x-api-key header.'], 'required': True}, 'app_name': {'description': ['(one of app_name or application_id ...

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/newrelic_deployment_v2.py:152:11: W292: no newline at end of file

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/newrelic_deployment_v2.py:152:0: missing-final-newline: Final newline missing

The test ansible-test sanity --test validate-modules [explain] failed with 9 errors:

plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'app_name' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'application_id' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'changelog' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'description' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'revision' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'token' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-missing-type: Argument 'user' in argument_spec uses default type ('str') but documentation doesn't define type
plugins/modules/newrelic_deployment_v2.py:0:0: doc-required-mismatch: Argument 'revision' in argument_spec is required, but is not documented as being required
plugins/modules/newrelic_deployment_v2.py:0:0: module-invalid-version-added: DOCUMENTATION: version_added ('0.1') is not a valid collection version (see specification at https://semver.org/): invalid semantic version '0.1'. Got {'module': 'newrelic_deployment_v2', 'version_added': '0.1', 'author': 'Davinder Pal (@116davinder)', 'short_description': 'Notify newrelic about app deployments using newrelic v2 api', 'description': ['Notify newrelic about app deployments (https://docs.newrelic.com/docs/apm/new-relic-apm/maintenance/record-deployments)'], 'options': {'token': {'description': ['API token, to place in the x-api-key header.'], 'required': True}, 'app_name': {'description': ['(one of app_name or application_id ...

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

plugins/modules/newrelic_deployment_v2.py:152:11: W292: no newline at end of file

click here for bot help

@ansibullbot ansibullbot added the ci_verified Push fixes to PR branch to re-run CI label Sep 9, 2020
@116davinder
Copy link
Author

116davinder commented Sep 10, 2020

@felixfontein / Community Team
I don't think NewRelic V1 is still there, I added this module 2 years back and Luckily Ansible Community wasn't interesting in adding new code back then.

If Ansible Community Guidelines is to overwrite v1 and keep module name same then I will update my PR to do that.
It will be just 5-10 mins task.

I have used this 3rd party module in production for more than 1 year with v2 name because earlier versions included default newrelic_deployment module.

@felixfontein
Copy link
Collaborator

Do you have links about the v1 deprecation / new v2 API? Also, is it possible to self-host NewRelic, and is it possible that someone is still using an old version which only has the v1 API?

If it is possible to adjust the existing module to work with the new API without breaking backwards compatibility (i.e. existing and working playbooks/roles continue to work) that's the preferred approach.

@116davinder
Copy link
Author

116davinder commented Sep 10, 2020

There is no exact date when v1 will be removed but it has been deprecated from more than 2 years and there is no development on it as per below deprecated notice link.

It's hard to support 2 API's in one module because they have different approaches. I believe it will more complication then gains. To maintain backward compatibility, I have introduced V2 name in module so everyone will know it's new module.

V1 is dead simple version: which supports both app_id and app_name as parameters.
V2 is bit complicated, app_id is only supported, if app_name is provided then we have use newrelic search api for app_id.

Ref:

I have hosted module in my private repository: 116davinder/ansible-custom-libs

@ansibullbot ansibullbot added monitoring and removed ci_verified Push fixes to PR branch to re-run CI labels Sep 10, 2020
@felixfontein
Copy link
Collaborator

Well, the important question is

is it possible to self-host NewRelic

and it looks like the answer is "no" (or if it is, the modules do not support it since the URL is hard-coded). On the other hand, in that case, why is there a validate_certs option?

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

It's too bad there's apparently no docs for the v1 API left - I would really like to know what's appname and how it relates to app_name. That seems to be only thing that cannot be set in the v2 API.

plugins/modules/monitoring/newrelic_deployment_v2.py Outdated Show resolved Hide resolved
@116davinder
Copy link
Author

is it possible to self-host NewRelic?
Sorry, I didn't understood last time. It can't be self hosted. NewRelic is Saas Platform.

@116davinder
Copy link
Author

@felixfontein, is there anything left to check in this PR?

@felixfontein
Copy link
Collaborator

I still think it makes more sense to update the existing module to use the new API, instead of adding a new module.

Pinging @mcodd the author and maintainer of the original newrelic_deployment module. Maybe he has an opinion on this.

@116davinder
Copy link
Author

I have tried contacting author but no response from more than 2 years on my original PR, so I believe, current community can take decision about it and move forward.

If you as reviewer suggests that I should update the existing code than that is also possible but I won't be able to make backward compatible because of no documentation of API v1 + I would have to add new variable where user have to specify which API version to use which I really don't want to do as it will add more complexity to simple module.

@116davinder
Copy link
Author

116davinder commented Sep 19, 2020

I already gave up on my Original PR because of No Response for more than 2 years, I think, this attempt / last attempt will go in vain as well.

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Sep 22, 2020
@felixfontein
Copy link
Collaborator

I don't think it will, because I'll just merge it if it looks reasonable and the original author doesn't react anymore.

@116davinder
Copy link
Author

@felixfontein any luck here?

@ansibullbot
Copy link
Collaborator

@!UNKNOWN @Akasurde @Alb0t @AnderEnder @ApsOps @AugustusKling @Aversiste @ColOfAbRiX @DataDog @DavidWittman @Deepakkothandan @DenBeke @FlossWare @GR360RY @GwenaelPellenArkeup @InTheCloudDan @JayKayy @Jmainguy @JoergFiedler @KKoukiou @KellerFuchs @KimNorgaard @L2G @LinusU @Lunik @MacLemon @MatrixCrawler @MeganLiu @Mogztter @MorrisA @NickatEpic @NilashishC @nitaco @Nosmoht @Qalthos @QijunPan @QuentinBrosse @RickS-C137 @Rylon @Sajna-Shetty @SamyCoenen @Sedward @Shaps @Slezhuk @Smithx10 @Spredzy @Tatsh @ThePixelDeveloper @timothyvandenbrande @TommyLike @Tomorrow9 @UnderGreen @Wolfant @aajdinov @aalexmonteiro @abarbare @abellotti @abulimov @adamgoossens @adamvaughan @adejoux @adriane-cardozo @adrianmoisey @agaffney @agmezr @ahtik @aimonb @akostyuk @albertomurillo @alcamie101 @alikins @alxgu @amasolov @aminvakil @amit0701 @andreparames @andrew-d @andsens @andyhky @andytom @angristan @angstwad @arturaz @azaghal @bachradsusi @baldwinSPC @bannaych @barnabycourt @barryib @bbyhuy @bcoca @bendoh @bennojoy @berenddeboer @berendt @bgaifullin @bgurney-rh @bhcopeland @bigmstone @billdodd @bincyber @bjolivot @bleader @bpennypacker @brampling @bregman-arie @brian-brazil @briceburg @brontitall @bsanders @bushvin @bvitnik @bwhaley @caphrim007 @carchi8py @catcombo @cben @cheese @chris93111 @chrishoffman @chrisisbeef @claco @clc-runner @clementtrebuchet @cloudnull @cmprescott @colin-nolan @commel @coreywan @cprh @cwollinger @d-little @danieljaouen @danielmellado @dankeder @dareko @davx8342 @dcermak @dch @decentral1se @derez @dermute @devyanikota @dinoocch @dirtyharrycallahan @displague @dj-wasabi @djmattyg007 @dkorn @dmtrs @dnix101 @dohoangkhiem @dougluce @dprts @drcapulet @drew-russell @drewkerrigan @drybjed @dstoflet @ebirn @edevenport @edisonxiang @eest @eikef @elad661 @elasticdog @enriclluelles @erjohnso @erydo @eryx12o45 @evertmulder @evgkrsk @evrardjp @fabulops @farhan7500 @fcuny @fgbulsoni @fishman @flaper87 @florianpaulhoberg @flynn1973 @fraff @freesky-edward @fxfitz @gamethis @ganeshrn @garbled1 @gautamphegde @genegr @getjack @gforster @giovannisciortino @giuseppe @glitchcrab @groks @gtanzillo @guillaume_ro_fr @haad @hekonsek @helldorado @hkariti @hnakamur @hnanni @hogarthj @hryamzik @huaweicloud @hulquest @hwDCN @ilicmilan @indrajitr @inetfuture @ivanvanderbyl @jagadeeshnv @jails @jairojunior @jake2184 @jamescassell @jasperla @jbenden @jcftang @jcgruenhage @jdauphant @jerome-quere @jhoekx @jirutka @jlaska @jle64 @johanwiren @john-westcott-iv @jose-delarosa @joshainglis @jparrill @jpdasma @jpmens @jsmartin @jsumners @jtyr @justjais @kahowell @kairoaraujo @kamsz @karmab @kavu @kbrebanov @keachi @kenichi-ogawa-1988 @kevensen @kindermoumoute @krisvasudevan @krsacme @kubevirt @kunkku @kyleabenson @lekum @levonet @lionmax @lmprice @lonico @lrupp @lukasbestle @m-yosefpor @machacekondra @madhav-bharadwaj @magnus919 @makaimc @manojmeda @marc-sensenich @markuman @marns93 @martinm82 @marvin-sinister @marwatk @matbu @matburt @mator @mattjeffery @mattupstate @matze @mavit @maxamillion @mcltn @mcodd @meerkampdvv @mekanix @melodous @mgedmin @mgruener @mheap @mmazur @molekuul @mpdehaan @mraineri @mross22 @mscherer @mulby @mwarkentin @n0trax @n0ts @nalsaber @nasx @nate-kingsley @natefoo @navalkp @nbuchwitz @ndavison @ndclt @ndswartz @nerzhul @neuroid @nibalizer @nitzmahone @niuzhenguo @noles @noseka1 @notok @nurfet-becirevic @obirvalger @oboukili @obourdon @ogenstad @oolongbrothers @opoplawski @opslounge @orgito @ovcharenko @overhacked @parfeon @pascalheraud @pb8226 @pertoft @phumpal @pilou- @pkliczewski @pmakowski @pmarkham @prabhosa @precurse @privateip @pubnub @pyykkis @quentinsf @quidame @raekins @rajeevarakkal @rambleraptor @ramondelafuente @ramooncamacho @ravibhure @remixtj @remyleone @ribbons @ricardogpsf @rmcintosh @robinro @robwagner33 @rohitChaware @rosmo @rosowiecki @rsmontero @russoz @rvalle @ryansb @sac @samdoran @saranyasridharan @sayap @scathatheworm @schmots1 @scodeman @scottanderson42 @sdodsley @seandst @sermilrod @sganesh-infoblox @sgargan @shane-walker @shirou @sieben @sile16 @sivel @sjaiswal @skinp @skornehl @slok @sm4rk0 @smashwilson @smbambling @snopoke @soodpr @srvg @steamx @stearz @stpierre @stygstra @stympy @supertom @suprememoocow @sysadmind @szinck @t0mk @t794104 @tacatac @talzur @tarka @tastychutney @tbielawa @tchernomax @tdtrask @teebes @tgoetheyn @thaumos @the-maldridge @thoiberg @tintoy @tksmd @tmiotto @tmshn @toabctl @tomasg2012 @tonyseek @treyperry @trishnaguha @troy2914 @tumbl3w33d @turb @tuxillo @tyouxa @tzure @ujwalkomarla @usawa @vaygr @vcarceler @vedit @verkaufer @vexata @vfauth @vincentvdk @vmalloc @vritant @waheedi @wbrefvem @weaselkeeper @willthames @willybarro @wltjr @wopfel @wtcross @xcambar @xen0l @xiaozhu36 @xorel @xuxiaowei0512 @yaacov @yanzhangi @yeukhon @zanssa @zbal @zengchen1024 @zgalor @zhhuta @zhongjun2 @zimbatm

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@felixfontein
Copy link
Collaborator

@116davinder if you update the existing module to use the new API, I'm happy to merge this if nobody complains. But I will not merge a new module next to the old one, just for a new API version.

* merged v2 code into v1 module
@ansibullbot ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR plugins plugin (any type) and removed community_review needs_triage new_module New module new_plugin New plugin stale_ci CI is older than 7 days, rerun before merging labels Dec 16, 2020
@ansibullbot
Copy link
Collaborator

@116davinder this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibullbot ansibullbot added feature This issue/PR relates to a feature request merge_commit This PR contains at least one merge commit. Please resolve! and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Dec 16, 2020
@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/monitoring/newrelic_deployment.py:148:0: missing-final-newline: Final newline missing

The test ansible-test sanity --test pylint [explain] failed with 1 error:

plugins/modules/monitoring/newrelic_deployment.py:148:0: missing-final-newline: Final newline missing

click here for bot help

@116davinder
Copy link
Author

116davinder commented Dec 16, 2020

created new request because of **** rebase issues.
#1501

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue/PR relates to a feature request has_issue merge_commit This PR contains at least one merge commit. Please resolve! module module monitoring needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants