[issue43756] About updating audit events when function gains new arguments

[Steve Dower]

Change by Steve Dower :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43756] About updating audit events when function gains new arguments

[Steve Dower]

Steve Dower  added the comment:


New changeset a32f8fe7133aad4f3cf8946534e3b79a5f2659da by Saiyang Gou in branch 
'master':
bpo-43756: Add new audit event for new arguments added to glob.glob (GH-25239)
https://github.com/python/cpython/commit/a32f8fe7133aad4f3cf8946534e3b79a5f2659da


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43756] About updating audit events when function gains new arguments

[Steve Dower]

Steve Dower  added the comment:

To remove an event, it needs to be deprecated as any public facing feature 
would be deprecated. I'm not sure how we'd go about publicising it though. 
There's currently no way to raise a warning, so it'd just have to be very 
clearly advertised in documentation (as a breaking change).

Easiest to assume that an event will be permanent. If there's a significant 
impact due to an event that we want to avoid, we can address it then.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43756] About updating audit events when function gains new arguments

[Saiyang Gou]

Saiyang Gou  added the comment:

And one more question, should we ever remove an old version of event when we 
add a new version of it? A function whose signature got repeatedly changed may 
result in several versions of audit event raised together on a single 
operation, which is probably not good for performance. But removing the old 
event will make those hooks which are not written to handle the new version of 
the event unable to receive the old event at all, which doesn't look good.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43756] About updating audit events when function gains new arguments

[Saiyang Gou]

Change by Saiyang Gou :


--
keywords: +patch
pull_requests: +23977
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/25239

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43756] About updating audit events when function gains new arguments

[Steve Dower]

Steve Dower  added the comment:

Solution 1 is definitely out, as PyArg_ParseTuple is a very valid way to 
implement hooks but won't handle longer tuples.

For Solution 2, we already have some events that use a slash to indicate 
"extra" information, so depending on context I'd either do "glob.glob/2" or add 
both "glob.glob/root_dir" and "glob.glob/dir_fd" events.

I suspect in this case, the "glob.glob/2" name is better, because all the data 
really has to come together. Sending those values through separately might make 
more sense if there's some other linkable id (such as for socket events).

Nobody should handle hooks that they don't expect with anything other than 
*args (and even then they should be careful, as hooks may pass through MB or GB 
of data, so logging or printing them directly may be a bad idea).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43756] About updating audit events when function gains new arguments

[Saiyang Gou]

New submission from Saiyang Gou :

I'm not sure what to do when a function already being audited gains new 
arguments which are also worth auditing. For example, issue 38144 brings new 
`root_dir` and `dir_fd` arguments to `glob.glob`. Currently `glob.glob` is 
already audited, with `(pathname, recursive)` as the event schema. However, the 
new `root_dir` and `dir_fd` arguments are important context for the glob 
operation and is also worth collecting into the event (as a comparison, `os.*` 
audit events already include `dir_fd`). The problem is how to incorporate the 
new arguments.

- Solution 1: Add new arguments to the existing event schema (on feature 
releases). In this ways, hooks expecting an arg tuple of the original length 
will be broken. Hooks taking arguments by subscripts (e.g. `pathname = 
args[0]`) may still work if we only append new arguments to the end of the 
event schema and never insert arguments in the middle.
- Solution 2: Add a new audit event. In this `glob` case, we add a new event 
which might be called `glob.glob.v2` or `glob.glob.3_10` or some other way. 
This will make sure the event schema is always fixed across different feature 
releases. But hooks may need to correctly handle all "versions" of events (e.g. 
do some thing like `if event.startswith('glob.glob')` instead of `if event == 
'glob.glob'`).

I guess more audited functions may gain new arguments in the future so this 
problem is unavoidable. I would like some suggestions from Steve. Thanks!

--
messages: 390386
nosy: gousaiyang, steve.dower
priority: normal
severity: normal
status: open
title: About updating audit events when function gains new arguments

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com