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

[IMP][16.0+] base_import_async: propagate context used during import #710

Open
AnizR opened this issue Nov 20, 2024 · 5 comments
Open

[IMP][16.0+] base_import_async: propagate context used during import #710

AnizR opened this issue Nov 20, 2024 · 5 comments

Comments

@AnizR
Copy link

AnizR commented Nov 20, 2024

Context

I was doing an asynchronous import on "account.asset" and realized that my assets were created but not visible.
After some investigation, I found that that it was caused by a field 'asset_type' that is set using the context key 'default_asset_type'

Proposition

I think that the context used during import should be propagate to the jobs created.
Or, at least, every key starting by 'default_...' .

It could be easily done by modifying the function on base_import.import:

def _job_prepare_context_before_enqueue_keys(self):

Question

Since I know that it could generate a debate, I opened this issue before creating any PR.

Should we propagate the context? All of it? A sub part of it?

@sbidoul
Copy link
Member

sbidoul commented Nov 20, 2024

This PR has some context: #432 (and links to much more context :)

@AnizR
Copy link
Author

AnizR commented Nov 20, 2024

This PR has some context: #432 (and links to much more context :)

Thanks! It gives more context but note that it refers to the "general" case and here, I would like to debate on the specific case of asynchronous import.

@sbidoul
Copy link
Member

sbidoul commented Nov 20, 2024

At first sight, in the specific case of base_import_async it makes sense to pass the same context that would be passed to a regular import.

@sbidoul
Copy link
Member

sbidoul commented Nov 20, 2024

There was #407, but this was apparently driven by the lang use case and was closed when lang went into the context ?

@amh-mw
Copy link
Contributor

amh-mw commented Nov 20, 2024

Also #613

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants