-
Notifications
You must be signed in to change notification settings - Fork 69
"a large number of fixes and cleanups" #1
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
Conversation
There's no need to do that
- Allow to change hostname from environment - Change to current API
This way we can know which documents errored and how many errors took place
This function returns information about the index, but also allows us to check if the index exists. This depends on another change in redigo - I might make a PR for that
|
Can this be merged? (this is the same as what you saw last week. All I did was add 'NOSTEM' support). |
|
@mnunberg the only thing I'm not sure about is the error map. How about this: we create our own error object, let's call it IndexingError or something, and we return that. If no error occurred - we return nil. If at least one document has erred - we return our error ojbect, and it contains a slice of sub errors (not a map since the access is by sequence anyway), and you can inspect that. i.e. type BulkIndexingError struct {
errors []error
}
func (e *BulkIndexingError)Error() string {
// yada yada
}
func (e *BulkIndexingError)Errors() []error {
return e.errors
}And thus we can keep the multi indexing function with the old signature which is more idiomatic func (i *Client) IndexOptions(opts IndexingOptions, docs ...Document) error {
} |
| RecordsPerDocAvg float64 `redis:"records_per_doc_avg"` | ||
| BytesPerRecordAvg float64 `redis:"bytes_per_record_avg"` | ||
| OffsetsPerTermAvg float64 `redis:"offsets_per_term_avg"` | ||
| OffsetBitsPerTermAvg float64 `redis:"offset_bits_per_record_avg"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will soon need the gc stats as well
|
Good idea about the error map, I'll get around to it soon. |
| // MultiError Represents one or more errors | ||
| type MultiError map[int]error | ||
|
|
||
| func (e MultiError) Error() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a blocker, but wouldn't you in this case rather chain all error strings somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't that require line breaks to make it readable, and wouldn't line breaks confuse a lot of things?
We can't really anticipate how many documents the user is putting in here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put newlines, we can always make it look like an array. It's just for debug printing etc.
| } | ||
|
|
||
| // MultiError Represents one or more errors | ||
| type MultiError map[int]error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather have a slice here where each sub error correlates to the index of the operation that triggered it
No description provided.