Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants