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

Add callback to global_config to allow tracking of one's own SDK usage #1469

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

haakonvt
Copy link
Contributor

@haakonvt haakonvt commented Nov 4, 2023

DRAFT

Ref. edm-pipeline debug discussion: @spex66 @thorkildcognite

Checklist:

  • Tests added/updated.
  • Documentation updated. Documentation is generated from docstrings - these must be updated according to your change.
    If a new method has been added it should be referenced in cognite.rst in order to generate docs based on its docstring.
  • Changelog updated in CHANGELOG.md.
  • Version bumped. If triggering a new release is desired, bump the version number in _version.py and pyproject.toml per semantic versioning.

@haakonvt haakonvt requested review from a team as code owners November 4, 2023 21:15
@haakonvt haakonvt marked this pull request as draft November 4, 2023 21:15
@spex66
Copy link

spex66 commented Nov 4, 2023

Not run it yet, but expected code consolidating counters across threads/processes?

Or does it require handling concurrency on callback side?

Parallelism still confuses me every time :)

@haakonvt haakonvt closed this Nov 5, 2023
@haakonvt
Copy link
Contributor Author

haakonvt commented Nov 5, 2023

Accidentally clicked close... 👀

@haakonvt haakonvt reopened this Nov 5, 2023
@haakonvt
Copy link
Contributor Author

haakonvt commented Nov 5, 2023

Not run it yet, but expected code consolidating counters across threads/processes?

@spex66 The callback will be called by the different threads executing the requests (the SDK only uses threading as a means of concurrency). In the two examples given in the docstring, they would all write to the same object, which is totally fine (GIL + ordering doesn't matter).

Do you have a usage pattern in mind that you're afraid might have a race condition?

@haakonvt haakonvt force-pushed the v7-release branch 6 times, most recently from 492960c to 0772ccc Compare November 14, 2023 08:27
Base automatically changed from v7-release to master November 14, 2023 10:50
@haakonvt
Copy link
Contributor Author

@spex66 do you need this feature or should we put it on hold?

@spex66
Copy link

spex66 commented Nov 29, 2023

Ok hold pls, and I need to set a reminder to look into it .. later

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