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.

Sign up for GitHub

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

Jump to bottom

Remove checks and callbacks from API #2164

Merged
merged 2 commits into from
Oct 12, 2021
Merged

Remove checks and callbacks from API #2164

merged 2 commits into from
Oct 12, 2021

Conversation

ocelotl
Copy link
Contributor

@ocelotl ocelotl commented Sep 29, 2021

Fixes #2151

@ocelotl ocelotl added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Sep 29, 2021
@ocelotl ocelotl requested a review from aabmass September 29, 2021 21:52
@ocelotl ocelotl self-assigned this Sep 29, 2021
@ocelotl ocelotl requested a review from a team September 29, 2021 21:52
@lzchen
Copy link
Contributor

lzchen commented Oct 1, 2021

@aabmass @ocelotl

I don't have the original context of the discussion (can someone link the discussion on the original PR)? Why are we removing the validation checks and callbacks?

@ocelotl
Copy link
Contributor Author

ocelotl commented Oct 1, 2021

Removing checks until we have figured out a mechanism for error handling as defined in #2148

lzchen
lzchen reviewed Oct 1, 2021
View reviewed changes
opentelemetry-api/src/opentelemetry/metrics/instrument.py

_name_regex = compile_(r"[a-zA-Z][-.\w]{0,62}")

@property
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the properties?

aabmass
aabmass approved these changes Oct 1, 2021
View reviewed changes
Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can we open up a follow up issue for the added todo/fixmes?

@ocelotl ocelotl merged commit 4254a0b into open-telemetry:metrics_new Oct 12, 2021
@ocelotl ocelotl deleted the issue_2151 branch October 12, 2021 14:20
ocelotl added a commit to ocelotl/opentelemetry-python that referenced this pull request Oct 13, 2021
@ocelotl
Remove checks and callbacks from API (open-telemetry#2164)
450a86c 
* Remove checks and callbacks from API

Fixes open-telemetry#2151

* Fix tests
@aabmass aabmass mentioned this pull request Oct 13, 2021
Closed
@aabmass aabmass mentioned this pull request Oct 29, 2021
Merged
aabmass pushed a commit to aabmass/opentelemetry-python that referenced this pull request Oct 29, 2021
@ocelotl @aabmass
Remove checks and callbacks from API (open-telemetry#2164)
2be0de4 
* Remove checks and callbacks from API

Fixes open-telemetry#2151

* Fix tests
ocelotl added a commit that referenced this pull request Nov 2, 2021
@ocelotl
Remove checks and callbacks from API (#2164)
0aa3eb6 
* Remove checks and callbacks from API

Fixes #2151

* Fix tests
@ocelotl ocelotl mentioned this pull request Dec 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lzchen lzchen lzchen approved these changes

@aabmass aabmass aabmass approved these changes

Assignees

@ocelotl ocelotl

Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ocelotl @lzchen @aabmass