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

Introduce $vcloud_log that logs into log/vcloud.log #16641

Merged
merged 1 commit into from
Dec 13, 2017

Conversation

miha-plesko
Copy link
Contributor

@miha-plesko miha-plesko commented Dec 12, 2017

With this commit we introduce yet another global logger that is to be used by VMware provider. We're having quite some logs there and it's better to log them into its own file.

@miq-bot add_label enhancement
@miq-bot assign @Fryguy

Related issue: #16642
BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1525201

/cc @blomquisg @gberginc

@miha-plesko miha-plesko changed the title Introduce $vmware_cloud_log that logs into log/vmware_cloud.log Introduce $vmware_log that logs into log/vmware.log Dec 12, 2017
@gberginc
Copy link
Contributor

LGTM

Reassigning to @blomquisg because he's been involved in the discussions.
@miq-bot assign @blomquisg

@miha-plesko Please, open an issue and mention that that's a request from Red Hat support team.

Adding labels for backports.
@miq-bot add_label gaprindashvili/yes, fine/yes

@chargio Please check that this is correct.

@chargio
Copy link
Contributor

chargio commented Dec 13, 2017

@blomquisg @agrare We need some guidelines here:

  • What should be the name of the log. Should it be independent on other VMware providers?
  • What information is in this log and what should be in production.log?

@agrare
Copy link
Member

agrare commented Dec 13, 2017

I think this should be called vcloud.log, we already have vim.log for vSphere (VIM is the name of the sdk) so vmware.log would be misleading. This also follows the pattern of other providers e.g. aws.log and azure.log

With this commit we introduce yet another global logger that is to be used by
VMware provider. We're having quite some logs there and it's better to log them
into its own file.

Signed-off-by: Miha Pleško <[email protected]>
@miha-plesko
Copy link
Contributor Author

I like vcloud better too, because we're not going to be using it for infra manager anyway. Thanks for noticing, I've repushed.

@miha-plesko miha-plesko changed the title Introduce $vmware_log that logs into log/vmware.log Introduce $vcloud_log that logs into log/vcloud.log Dec 13, 2017
@miq-bot
Copy link
Member

miq-bot commented Dec 13, 2017

Checked commit miha-plesko@cb112a8 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. ⭐

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

LGTM

@agrare agrare merged commit d144043 into ManageIQ:master Dec 13, 2017
@agrare agrare added this to the Sprint 76 Ending Jan 1, 2018 milestone Dec 13, 2017
@agrare
Copy link
Member

agrare commented Dec 13, 2017

I'm going to remove the backport labels since this is an RFE and doesn't seem to be critical

@miha-plesko
Copy link
Contributor Author

@agrare thanks for merging. Could you also take a look at #16455? Exactly same thing, only for Nuage provider.

@agrare
Copy link
Member

agrare commented Dec 13, 2017

@miha-plesko no problem :)
There is an open question to Jason on that one that hasn't been resolved

@chargio
Copy link
Contributor

chargio commented Dec 14, 2017

@agrare The request for this comes from Support. They need the logs to be able to support properly the provider.

So I would add the labels back if you don't mind 😄 , we want them backported

@blomquisg
Copy link
Member

Adding back the backport labels. This is specific for a Fine release.

simaishi pushed a commit that referenced this pull request Dec 19, 2017
Introduce $vcloud_log that logs into log/vcloud.log
(cherry picked from commit d144043)

https://bugzilla.redhat.com/show_bug.cgi?id=1527555
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit e37bb361a6b2a174ec46d42ab58686e655395203
Author: Adam Grare <[email protected]>
Date:   Wed Dec 13 12:18:09 2017 -0500

    Merge pull request #16641 from miha-plesko/vcd-logger
    
    Introduce $vcloud_log that logs into log/vcloud.log
    (cherry picked from commit d14404370304efc0e5b1243a750e2c8da03ed4e4)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1527555

@simaishi
Copy link
Contributor

simaishi commented Jan 4, 2018

@miha-plesko Please create a separate PR for Fine branch. The logging has changed in master in #15397 and there are conflicts backporting this to Fine branch.

miha-plesko added a commit to miha-plesko/manageiq that referenced this pull request Jan 5, 2018
There is a [PR](ManageIQ#16641)
already merged in master that introduces $vcloud_log, but since
logging has changed in this [PR](ManageIQ#15397),
there are merge conflicts for fine branch. Hence this commit.

Signed-off-by: Miha Pleško <[email protected]>
@miha-plesko
Copy link
Contributor Author

Done, I've assigned you on the fine PR. Probably we will also want to backport this vmware PR to actually make use of the new $vcloud_log.

@simaishi
Copy link
Contributor

simaishi commented Jan 5, 2018

Backported to Fine via #16746

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
There is a [PR](ManageIQ#16641)
already merged in master that introduces $vcloud_log, but since
logging has changed in this [PR](ManageIQ#15397),
there are merge conflicts for fine branch. Hence this commit.

Signed-off-by: Miha Pleško <[email protected]>
@miha-plesko miha-plesko deleted the vcd-logger branch January 7, 2019 08:22
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.

8 participants