Skip to content

Conversation

vtemian
Copy link
Contributor

@vtemian vtemian commented Nov 12, 2014

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

we should move these raven imports inside if args.sentry_dsn: so we do not have hard dependency on raven module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but if you install it from ppa and built it with pex, it would pe harder to work like that

@calind calind assigned vtemian and unassigned calind Nov 12, 2014
gitfs/router.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

We should drop socket.gethostname() as it's a rather expensive syscall. If it's configured for raven i think it does this automatically, or if it doesn't we should use http://raven.readthedocs.org/en/latest/usage.html#raven.base.Client.context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that raven do it by default and if socket.gethostname is to
expensive, we could use os.environ["HOSTNAME"]. If we don't append it in
message and send it using context or tags, we still need to get the
hostname

On Wed, Nov 12, 2014 at 7:28 PM, Calin Don notifications@github.com wrote:

In gitfs/router.py:

@@ -126,7 +127,13 @@ def call(self, operation, *args):
view.class.name))
raise FuseOSError(ENOSYS)

  •    return getattr(view, operation)(*args)
    
  •    try:
    
  •        return getattr(view, operation)(*args)
    
  •    except FuseOSError as e:
    
  •        raise e
    
  •    except Exception as exception:
    
  •        log.exception("[%s] A system call failed" % socket.gethostname())
    

We should drop socket.gethostname() as it's a rather expensive syscall.
If it's configured for raven i think it does this automatically, or if it
doesn't we should use
http://raven.readthedocs.org/en/latest/usage.html#raven.base.Client.context


Reply to this email directly or view it on GitHub
https://github.com/PressLabs/gitfs/pull/162/files#r20234633.

Vlad

Copy link
Contributor

Choose a reason for hiding this comment

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

Just checked. raven and it automatically adds the server. Check http://sentry.presslabs.net/production/gitfs/group/692/ right column.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool

On Wed, Nov 12, 2014 at 7:40 PM, Calin Don notifications@github.com wrote:

In gitfs/router.py:

@@ -126,7 +127,13 @@ def call(self, operation, *args):
view.class.name))
raise FuseOSError(ENOSYS)

  •    return getattr(view, operation)(*args)
    
  •    try:
    
  •        return getattr(view, operation)(*args)
    
  •    except FuseOSError as e:
    
  •        raise e
    
  •    except Exception as exception:
    
  •        log.exception("[%s] A system call failed" % socket.gethostname())
    

Just checked. raven and it automatically adds the server. Check
http://sentry.presslabs.net/production/gitfs/group/692/ right column.


Reply to this email directly or view it on GitHub
https://github.com/PressLabs/gitfs/pull/162/files#r20235478.

Vlad

Copy link
Contributor

Choose a reason for hiding this comment

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

Also check the above comment.

calind added a commit that referenced this pull request Nov 13, 2014
@calind calind merged commit 6e0a77f into master Nov 13, 2014
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.

3 participants