-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add context to load_csv function #13
Conversation
result = ctx.env[model_name].load(head, values) | ||
model = ctx.env[model_name] | ||
if context: | ||
model = model.with_context(context) |
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 think it would be better to allow to pass a model
instead of a model_name
(supporting both for backward-compatibility), thus, you can do whatever you want such as load_csv_stream(ctx, ctx.env['my.model'].sudo(5).with_context(x=y), data)
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.
Ok I'll change that, wasn't sure if we wanted to keep a string or if passing a model would be more suited.
d505b3d
to
03571df
Compare
@@ -51,16 +51,18 @@ def load_csv_stream(ctx, model_name, data, dialect='excel', encoding='utf-8', | |||
head = data.next() | |||
values = list(data) | |||
if values: | |||
result = ctx.env[model_name].load(head, values) | |||
if isinstance(model, str): |
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.
s/str/basestring/
because unicode
is not str
in order to be able to pass a context like a specific language using with_context on the model
…rsions This fix an error with PIL during tests
03571df
to
26cbce5
Compare
fixed isinstance and squashed commits |
in order to be able to pass a context like a specific language