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

[WIP] Add rough draft of configuration options documentation #373

Merged
merged 8 commits into from
Feb 9, 2016

Conversation

Crisfole
Copy link
Contributor

@Crisfole Crisfole commented Feb 2, 2016

@jondeandres @brianr

This is a first pass at each one. I stuck a few TODO where I was extremely uncertain, and it's not organized the way I want yet (see the TODO at the top).


## async_handler

If `config.use_async = true` explicitly sets the function used to send
Copy link
Contributor

Choose a reason for hiding this comment

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

The object passed to this option should respond to #call and receive the payload. Examples in lib/rollbar/delay/.

@jondeandres
Copy link
Contributor

Nice work @Crisfole 😄. I just wrote some inline comments in the docs.

## scrub_password

TODO: CORRECT THIS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

These values should be boolean and the purpose is what you have already there.

When scrubbing the URL, we scrub or not the user/password. By default true.

So, scrubbing URLs, we always scrub the query string, and the user and password is configurable. We'll scrub by default those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Who sends their password in the URL!? Or does this scrub access tokens too?

That's not hidden by HTTPS...If they're sending the password in plaintext in the URL they've got bigger problems that us recording it in Rollbar!

@Crisfole
Copy link
Contributor Author

Crisfole commented Feb 3, 2016

I'm still not 100% certain I have all this right.

@jondeandres

  • What’s the difference between delayed_job_enabled and report_dj_data?
  • What about the person_method*? Is it really rails only? Or does it accept anything that responds to call?
  • Can someone go over this in detail? I know it's a pain, but it's the only way to make sure we're not missing anything obvious.

@jondeandres
Copy link
Contributor

@Crisfole if delayed_job_enabled is enabled we'll wrap the delayed_job worker so the errors in the jobs are reported.

Instead of this, there is some delayed_job data to be sent or not. We can disabled it with report_dj_data = false. So, the job itself has some metadata related, that is the data to be sent or not.

@Crisfole
Copy link
Contributor Author

Crisfole commented Feb 5, 2016

@jondeandres what sort of metadata are we talking?

@jondeandres
Copy link
Contributor

@Crisfole job class, queue...


### branch

**Default** "master"
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at our code I cannot see a default master value for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default is master on the server side.

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'll toss it anyway.

**Default** `false`

When `true` indicates you wish to send data to Rollbar asynchronously. If
installed, uses `girl_friday`, otherwise defaults to `Threading`.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use the ` characters you should use existing methods, constants, etc...

In ruby Threading is not a constant and doesn't exist in the core library, use Thread instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, didn't know the name of ruby's Thread implementation, just sort of typed stuff in.

@Crisfole
Copy link
Contributor Author

Crisfole commented Feb 8, 2016

@jondeandres All set, I believe.

jondeandres added a commit that referenced this pull request Feb 9, 2016
[WIP] Add rough draft of configuration options documentation
@jondeandres jondeandres merged commit 1171b8a into rollbar:master Feb 9, 2016
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.

2 participants