PR for #170: let the window option choose a window range

Closes #170

@Constantino-Carreto-Romero If I run
xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(max)
with the test data I get a no observations error. This is counterintuive since
xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(max)
does run. So I would think the maximum window in this setting is at least 5. There's no imputation here, is there a bug when calculating the max window without impute?

@Constantino-Carreto-Romero I get an error when running
xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(max) impute(stag) trend(-2)
This happens because the check for whether the trend starting point is in the estimation window is using the values of lwindow and rwindow and at the time of the check these are strings. Maybe the check needs to be later, and it must use the calculated window limits.

@Constantino-Carreto-Romero If I run
xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(balanced) impute(stag)
I get an error message. The found window limits are -1 and -1.
This doesn't sem right. If we run
xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(balanced) impute(stag) savek(event)
bys i: egen min = min(event_evtime)
bys i: egen max = max(event_evtime)
preserve
bys i: keep if _n == 1
tab min
tab max
restore

We get that all treated units have at least two periods pretreatment and one period post treatment (the treatment period, 0). So the balanced window should be -2, 0. Why is it not the case? Am I misreading how the balanced window should be calculated?

@Constantino-Carreto-Romero please run more checks interacting the new window options and the other options in xtevent. The fact that I was able to find clashes when using trend or using the balanced option makes me think that we need more checking before we are ready to merge.

@Constantino-Carreto-Romero  Note changes in #181 to avoid conflicts when merging this PR

@Constantino-Carreto-Romero If I run xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(max) with the test data I get a no observations error. This is counterintuive since xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(5) does run. So I would think the maximum window in this setting is at least 5. There's no imputation here, is there a bug when calculating the max window without impute?

@jorpppp thanks
If I run the following example
use example31.dta, clear
xtset i t
*generate standardized time variable
gen diffpolicyvar=d1.z
gen timeoftreatment=t if diffpolicyvar==1
by i: egen ttreat= min(timeoftreatment)
gen t_relative = t - ttreat
*tabulate the standardized time variable
tab t_relative

when tabulating the standardized time variable I get the following. It indicates that the maximum range of estimation is -19, 17. But then, adjusting for the endpoints, the maximum possible window the user could specify is window(-18 16). Internally, the program tries to estimate using that window. Nonetheless, since impute is not active, the event-time dummies are generated with missing values. For instance, running the following
xtevent y, panelvar(i) timevar(t) policyvar(z) w(-18 16) savek(aa, noestimate)

and then taking a look at the event-time dummies, or checking
count if !missing(aa_eq_m19) & !missing(aa_eq_p17)

it returns 0. Therefore, no observation succeeds in having non-missing values for all the event-time dummies.
Now if I use the eventdd command, I get the following result. By default, eventdd estimates the maximum possible window, in this case, 19 leads and 17 lags (eventdd does not differentiate the endpoints from the rest of event-time dummies). eventdd succeeds at estimating because by default it assumes staggered adoption and therefore it imputes the event-time dummies and endpoints. If I now try xtevent with the impute option:
xtevent y, panelvar(i) timevar(t) policyvar(z) w(-18 16) impute(stag)

I get the same estimation range as with eventdd. Therefore, in the case of xtevent, with the default of not activating the impute option, success in the estimation will depend on how many event-time dummies coefficients it has to estimate, according to the calculated window. If the dataset has a short observed time range, the maximum possible window will likely be short enough such that even without the impute option, the program will complete the estimation.

 This is counterintuive since xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(5) does run. So I would think the maximum window in this setting is at least 5. There's no imputation here, is there a bug when calculating the max window without impute?

@jorpppp thanks If I run the following example
use example31.dta, clear
xtset i t
*generate standardized time variable
gen diffpolicyvar=d1.z
gen timeoftreatment=t if diffpolicyvar==1
by i: egen ttreat= min(timeoftreatment)
gen t_relative = t - ttreat
*tabulate the standardized time variable
tab t_relative

when tabulating the standardized time variable I get the following. It indicates that the maximum range of estimation is -19, 17. But then, adjusting for the endpoints, the maximum possible window the user could specify is window(-18 16). Internally, the program tries to estimate using that window. Nonetheless, since impute is not active, the event-time dummies are generated with missing values. For instance, running the following
xtevent y, panelvar(i) timevar(t) policyvar(z) w(-18 16) savek(aa, noestimate)

and then taking a look at the event-time dummies, or checking
count if !missing(aa_eq_m19) & !missing(aa_eq_p17)

it returns 0. Therefore, no observation succeeds in having non-missing values for all the event-time dummies. Now if I use the eventdd command, I get the following result. By default, eventdd estimates the maximum possible window, in this case, 19 leads and 17 lags (eventdd does not differentiate the endpoints from the rest of event-time dummies). eventdd succeeds at estimating because by default it assumes staggered adoption and therefore it imputes the event-time dummies and endpoints. If I now try xtevent with the impute option:
xtevent y, panelvar(i) timevar(t) policyvar(z) w(-18 16) impute(stag)

I get the same estimation range as with eventdd. Therefore, in the case of xtevent, with the default of not activating the impute option, success in the estimation will depend on how many event-time dummies coefficients it has to estimate, according to the calculated window. If the dataset has a short observed time range, the maximum possible window will likely be short enough such that even without the impute option, the program will complete the estimation.

@Constantino-Carreto-Romero What I am reading from this is that the criteria to select the maximum window that we are using only works for staggered adoption, and that we need to figure out a criterion that works outside the staggered adoption case. Can you figure out what the criterion is?

 This is counterintuive since xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(5) does run. So I would think the maximum window in this setting is at least 5. There's no imputation here, is there a bug when calculating the max window without impute?

@jorpppp thanks If I run the following example
use example31.dta, clear
xtset i t
*generate standardized time variable
gen diffpolicyvar=d1.z
gen timeoftreatment=t if diffpolicyvar==1
by i: egen ttreat= min(timeoftreatment)
gen t_relative = t - ttreat
*tabulate the standardized time variable
tab t_relative

when tabulating the standardized time variable I get the following. It indicates that the maximum range of estimation is -19, 17. But then, adjusting for the endpoints, the maximum possible window the user could specify is window(-18 16). Internally, the program tries to estimate using that window. Nonetheless, since impute is not active, the event-time dummies are generated with missing values. For instance, running the following
xtevent y, panelvar(i) timevar(t) policyvar(z) w(-18 16) savek(aa, noestimate)

and then taking a look at the event-time dummies, or checking
count if !missing(aa_eq_m19) & !missing(aa_eq_p17)

it returns 0. Therefore, no observation succeeds in having non-missing values for all the event-time dummies. Now if I use the eventdd command, I get the following result. By default, eventdd estimates the maximum possible window, in this case, 19 leads and 17 lags (eventdd does not differentiate the endpoints from the rest of event-time dummies). eventdd succeeds at estimating because by default it assumes staggered adoption and therefore it imputes the event-time dummies and endpoints. If I now try xtevent with the impute option:
xtevent y, panelvar(i) timevar(t) policyvar(z) w(-18 16) impute(stag)

I get the same estimation range as with eventdd. Therefore, in the case of xtevent, with the default of not activating the impute option, success in the estimation will depend on how many event-time dummies coefficients it has to estimate, according to the calculated window. If the dataset has a short observed time range, the maximum possible window will likely be short enough such that even without the impute option, the program will complete the estimation.

@Constantino-Carreto-Romero What I am reading from this is that the criteria to select the maximum window that we are using only works for staggered adoption, and that we need to figure out a criterion that works outside the staggered adoption case. Can you figure out what the criterion is?

Per call: we won't try to deal with the case of automatic window choice outside staggered adoption. @Constantino-Carreto-Romero

@Constantino-Carreto-Romero we won't deal with the case outside staggered adoption. So the automatic window generation commands should check for staggered adoption before proceding, and return an error otherwise.
The other open comments in this PR are also pending.

@Constantino-Carreto-Romero Keep the changes in #187 in mind when merging this PR

@Constantino-Carreto-Romero I get an error when running xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(max) impute(stag) trend(-2) This happens because the check for whether the trend starting point is in the estimation window is using the values of lwindow and rwindow and at the time of the check these are strings. Maybe the check needs to be later, and it must use the calculated window limits.

@jorpppp
In https://github.com/JMSLab/xtevent/commit/de25d54a0c276cce974e52e8fce47720d166f612 and https://github.com/JMSLab/xtevent/commit/9fcf9b35c789fbda477e942821b8195d2df2b2f4 I kept the check in _eventols but conditioning that window must be numeric. Then, in _eventgenvars, once the window is calculated (if window(max | balanced) was specified), the same check is carried out. The check must be placed in _eventgenvars, before the code that specifies the coefficients that will be normalized.
I kept the check in _eventols for the numeric window case, so the program can stop at that instance and not let it stop until _eventgenvars. Should I change it to a single check in _eventgenvars?

@Constantino-Carreto-Romero I get an error when running xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(max) impute(stag) trend(-2) This happens because the check for whether the trend starting point is in the estimation window is using the values of lwindow and rwindow and at the time of the check these are strings. Maybe the check needs to be later, and it must use the calculated window limits.

@jorpppp In de25d54 and 9fcf9b3 I kept the check in _eventols but conditioning that window must be numeric. Then, in _eventgenvars, once the window is calculated (if window(max | balanced) was specified), the same check is carried out. The check must be placed in _eventgenvars, before the code that specifies the coefficients that will be normalized. I kept the check in _eventols for the numeric window case, so the program can stop at that instance and not let it stop until _eventgenvars. Should I change it to a single check in _eventgenvars?

@Constantino-Carreto-Romero Thanks. It's ok to keep the check in both places.

@Constantino-Carreto-Romero If I run xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(balanced) impute(stag) I get an error message. The found window limits are -1 and -1.
This doesn't seem right. If we run
xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(balanced) impute(stag) savek(event)
bys i: egen min = min(event_evtime)
bys i: egen max = max(event_evtime)
preserve
bys i: keep if _n == 1
tab min
tab max
restore

We get that all treated units have at least two periods pretreatment and one period post treatment (the treatment period, 0). So the balanced window should be -2, 0. Why is it not the case? Am I misreading how the balanced window should be calculated?

@jorpppp The calculated window is (-1 -1) plus the endpoints (-2+, 0+). The calculated limits are indeed (-2 0) but I follow the definition used for a numeric input for window and separate the periods covered by window and the endpoints. After the changes in 267cef7, running the same example produces this more informative error message.

@Constantino-Carreto-Romero Thanks. The error message is still a bit confusing. Maybe write "the calculated window is (-1,-1), excluding the endpoints". The -2+, 0+ is confusing, I don't know what the plus signs mean. So I would omit it.

@jorpppp
I edited the message in https://github.com/JMSLab/xtevent/commit/8409db79fc408499f779215ade1eb9fe0f43f28b. Is it ok?

In testing this I found a possible bug: xtevent y, w(0 3) pol(z) impute(stag) returns an error with the example31 data. Can you check please?

I tested it and I got the error message. The error comes from this line https://github.com/JMSLab/xtevent/blob/8409db79fc408499f779215ade1eb9fe0f43f28b/xtevent/_eventgenvars.ado#L454 where the temporary variable being analyzed was not defined because this code section https://github.com/JMSLab/xtevent/blob/8409db79fc408499f779215ade1eb9fe0f43f28b/xtevent/_eventgenvars.ado#L430 was not executed because the left window is 0. I'm not sure whether it is a bug. It is not related to the automatic-window implementation, can I open a separate issue to check it?

@Constantino-Carreto-Romero If I run xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(balanced) impute(stag) I get an error message. The found window limits are -1 and -1.
This doesn't seem right. If we run
xtevent y eta, panelvar(i) timevar(t) policyvar(z) w(balanced) impute(stag) savek(event)
bys i: egen min = min(event_evtime)
bys i: egen max = max(event_evtime)
preserve
bys i: keep if _n == 1
tab min
tab max
restore

We get that all treated units have at least two periods pretreatment and one period post treatment (the treatment period, 0). So the balanced window should be -2, 0. Why is it not the case? Am I misreading how the balanced window should be calculated?

@jorpppp The calculated window is (-1 -1) plus the endpoints (-2+, 0+). The calculated limits are indeed (-2 0) but I follow the definition used for a numeric input for window and separate the periods covered by window and the endpoints. After the changes in 267cef7, running the same example produces this more informative error message.

@Constantino-Carreto-Romero Thanks. The error message is still a bit confusing. Maybe write "the calculated window is (-1,-1), excluding the endpoints". The -2+, 0+ is confusing, I don't know what the plus signs mean. So I would omit it.

@jorpppp I edited the message in 8409db7. Is it ok?

@Constantino-Carreto-Romero let me make a minor change to it in a push. After that I'll test the code a bit more and if it's ok I'll merge the PR.

In testing this I found a possible bug: xtevent y, w(0 3) pol(z) impute(stag) returns an error with the example31 data. Can you check please?

I tested it and I got the error message. The error comes from this line
https://github.com/JMSLab/xtevent/blob/8409db79fc408499f779215ade1eb9fe0f43f28b/xtevent/_eventgenvars.ado#L454
where the temporary variable being analyzed was not defined because this code section
https://github.com/JMSLab/xtevent/blob/8409db79fc408499f779215ade1eb9fe0f43f28b/xtevent/_eventgenvars.ado#L430
was not executed because the left window is 0. I'm not sure whether it is a bug. It is not related to the automatic-window implementation, can I open a separate issue to check it?

@Constantino-Carreto-Romero Yes, please open a new issue for this.

@Constantino-Carreto-Romero We still have bugs in this. With the example31 data:
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

returns an error message. I presume touse is not being set somewhere and the calculated window is using the entire sample and not the touse sample. Note that if we pass a sample restriction to xtevent this works:
xtevent y eta i.pois if pois!=., panelvar(i) timevar(t) pol(z) window(max) impute(stag)

Can you check this please?

@Constantino-Carreto-Romero We still have bugs in this. With the example31 data:
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

returns an error message. I presume touse is not being set somewhere and the calculated window is using the entire sample and not the touse sample. Note that if we pass a sample restriction to xtevent this works:
xtevent y eta i.pois if pois!=., panelvar(i) timevar(t) pol(z) window(max) impute(stag)

Can you check this please?

@jorpppp I tried running the same example but cannot replicate the error message. When running
use example31, clear
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

I get the following outcome.

@Constantino-Carreto-Romero did not get the error because he wasn't trying to plot. Strictly speaking the return message appears with
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag) plot

In any case there is a bug because the calculated window is not taking the missing values of the control variables into account, but it should. The issue has to do with the way samples are marked throughout: _eventgenvars is working with a sample that has not been marked for missing values of the variables, which is convenient for other reasons, e.g. imputation. @Constantino-Carreto-Romero will check this.

@Constantino-Carreto-Romero We still have bugs in this. With the example31 data:
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

returns an error message. I presume touse is not being set somewhere and the calculated window is using the entire sample and not the touse sample. Note that if we pass a sample restriction to xtevent this works:
xtevent y eta i.pois if pois!=., panelvar(i) timevar(t) pol(z) window(max) impute(stag)

Can you check this please?

@jorpppp I tried running the same example but cannot replicate the error message. When running
use example31, clear
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

I get the following outcome.

@Constantino-Carreto-Romero did not get the error because he wasn't trying to plot. Strictly speaking the error message appears with
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag) plot

In any case there is a bug because the calculated window is not taking the missing values of the control variables into account, but it should. The issue has to do with the way samples are marked throughout: _eventgenvars is working with a sample that has not been marked for missing values of the variables, which is convenient for other reasons, e.g. imputation. @Constantino-Carreto-Romero will check this.

@jorpppp
in https://github.com/JMSLab/xtevent/commit/6dcd33c45d6b77922f60bd2c02dc4d84e707fb2e I modified the syntax of _eventgenvars, so it can handle the local varlist. Then, I added a marksample to mark the non-missing observations in varlist and consider this restriction when calculating the window limits. This doesn't affect the already defined mark because the latter only considers if or in restrictions. I also updated the calls to _eventgenvars from other programs.  I include in the do-file in the issue folder examples to test that:

the calculation of the window limits considers missing values in the local varlist
the consideration of if or in conditions were not affected

@jorpppp
in https://github.com/JMSLab/xtevent/pull/178/commits/97dff856316fb4f9fcdb8abad2016b63f0a46791 I uploaded an issue folder with a do-file with examples to test: i) error messages; ii) correct functionality; iii) interaction with other options and iv; that the implementation doesn't alter other functionalities.

@Constantino-Carreto-Romero We still have bugs in this. With the example31 data:
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

returns an error message. I presume touse is not being set somewhere and the calculated window is using the entire sample and not the touse sample. Note that if we pass a sample restriction to xtevent this works:
xtevent y eta i.pois if pois!=., panelvar(i) timevar(t) pol(z) window(max) impute(stag)

Can you check this please?

@jorpppp I tried running the same example but cannot replicate the error message. When running
use example31, clear
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

I get the following outcome.

@Constantino-Carreto-Romero did not get the error because he wasn't trying to plot. Strictly speaking the error message appears with
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag) plot

In any case there is a bug because the calculated window is not taking the missing values of the control variables into account, but it should. The issue has to do with the way samples are marked throughout: _eventgenvars is working with a sample that has not been marked for missing values of the variables, which is convenient for other reasons, e.g. imputation. @Constantino-Carreto-Romero will check this.

@jorpppp in 6dcd33c I modified the syntax of _eventgenvars, so it can handle the local varlist. Then, I added a marksample to mark the non-missing observations in varlist and consider this restriction when calculating the window limits. This doesn't affect the already defined mark because the latter only considers if or in restrictions. I also updated the calls to _eventgenvars from other programs. I include in the do-file in the issue folder examples to test that:

the calculation of the window limits considers missing values in the local varlist
the consideration of if or in conditions were not affected

examples:
https://github.com/JMSLab/xtevent/blob/97dff856316fb4f9fcdb8abad2016b63f0a46791/issue170/issue170.do#L78

Thanks @Constantino-Carreto-Romero . This approach works but has a slight disadvantage. _eventgenvars is recalculating the marked sample that considers missing values in the variables. But this marked sample was already calculated by _eventols or _eventiv.
So instead of passing the varlist to _eventgenvars to repeat the marking, we can just pass the marked sample variable from _eventols or _eventiv to eventgenvars as an option.

@Constantino-Carreto-Romero We still have bugs in this. With the example31 data:
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

returns an error message. I presume touse is not being set somewhere and the calculated window is using the entire sample and not the touse sample. Note that if we pass a sample restriction to xtevent this works:
xtevent y eta i.pois if pois!=., panelvar(i) timevar(t) pol(z) window(max) impute(stag)

Can you check this please?

@jorpppp I tried running the same example but cannot replicate the error message. When running
use example31, clear
gen pois = rpoisson(5) in 1/200
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag)

I get the following outcome.

@Constantino-Carreto-Romero did not get the error because he wasn't trying to plot. Strictly speaking the error message appears with
xtevent y eta i.pois, panelvar(i) timevar(t) pol(z) window(max) impute(stag) plot

In any case there is a bug because the calculated window is not taking the missing values of the control variables into account, but it should. The issue has to do with the way samples are marked throughout: _eventgenvars is working with a sample that has not been marked for missing values of the variables, which is convenient for other reasons, e.g. imputation. @Constantino-Carreto-Romero will check this.

@jorpppp in 6dcd33c I modified the syntax of _eventgenvars, so it can handle the local varlist. Then, I added a marksample to mark the non-missing observations in varlist and consider this restriction when calculating the window limits. This doesn't affect the already defined mark because the latter only considers if or in restrictions. I also updated the calls to _eventgenvars from other programs. I include in the do-file in the issue folder examples to test that:

the calculation of the window limits considers missing values in the local varlist
the consideration of if or in conditions were not affected

examples:
https://github.com/JMSLab/xtevent/blob/97dff856316fb4f9fcdb8abad2016b63f0a46791/issue170/issue170.do#L78

Thanks @Constantino-Carreto-Romero . This approach works but has a slight disadvantage. _eventgenvars is recalculating the marked sample that considers missing values in the variables. But this marked sample was already calculated by _eventols or _eventiv.
So instead of passing the varlist to _eventgenvars to repeat the marking, we can just pass the marked sample variable from _eventols or _eventiv to eventgenvars as an option.

Thanks @Constantino-Carreto-Romero . This approach works but has a slight disadvantage. _eventgenvars is recalculating the marked sample that considers missing values in the variables. But this marked sample was already calculated by _eventols or _eventiv.
So instead of passing the varlist to _eventgenvars to repeat the marking, we can just pass the marked sample variable from _eventols or _eventiv to _eventgenvars as an option.

@jorpppp
thanks. In https://github.com/JMSLab/xtevent/commit/d7ae4e6ee6bb3a31b238ac27ab48799071062b5e I reverted the changes to _eventgenvars's syntax and instead of, I passed the marked sample variable from _eventols, _eventiv, etc. as an option in _eventgenvars. Then, I ran the do-file in the issue folder to check that this change worked.

When running
use example31, clear
xtevent y eta, panelvar(i) timevar(t) pol(z) impute(stag) window(max) diffavg

There is a bug because we get a calculated left endpoint of -19, but the computation of the differences of averages only includes until coefficient -9.


Question: ComputationField: want datetime difference

Hello
I would like to make a report (time series report?) showing average delay (stop - start datetime) per item_type between two dates, grouped by week.
All the info is present in the model, like start, stop, item_type.
I already have a report showing total of items (per type) over time and was hoping to use a custom ComputationField (or the used computation method), but I get nowhere.
Any example much appreciated...
best regards
gilles
PS:
I added a property in the model, and it is usefull when displaying a table, but it is not taken in consideration by slick reportin and result in ' Cannot resolve keyword 'availability_delay' into field' error
My model is like:
"""
class Product(models.Model):
zone = models.CharField(max_length=40, blank=False, null=False)
creation_date = models.DateTimeField(blank=True, null=True)
discovered_at = models.DateTimeField(blank=True, null=True, default=datetime.now)
class Meta:
    app_label = 'mod_asp'
    managed = True
    db_table = 'product'

@property
def availability_delay(self):
    return self.discovered_at - self.creation_date

"""

Hello
Slick reporting uses database fields to compute the results, and not model properties.
For this case, you would need to either

create a "materialized" column containing the date/time difference you want in the database table, and use it in the reports,
OR
create a custom computation fiield as in https://django-slick-reporting.readthedocs.io/en/latest/topics/computation_field.html

Hope this helps and i apologize for the late reply
Regards


Feat remove  code style rules in security practices

Remove  code style rules in security practices, such as tab indent.

Codecov Report

Merging #515 into release/0.10.0 will not change coverage.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           release/0.10.0     #515   +/-   ##
===============================================
  Coverage           79.35%   79.35%
===============================================
  Files                   9        9
  Lines                 436      436
  Branches               83       83
===============================================
  Hits                  346      346
  Misses                 88       88
  Partials                2        2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 792958f...2259655. Read the comment docs.


_publish_to_broker() is defined twice in crawler/emitter.py

https://github.com/cloudviz/agentless-system-crawler/blob/master/crawler/emitter.py#L282
flake8 . --count --exit-zero --max-line-length=127 --statistics | grep F811

@ricarkol possibly a latent merge error?

@ricarkol possibly a latent merge error?

removed the extra copy


Fix typos

Good catch, thank you!

:partying_face: :partying_face: :partying_face: :partying_face: :partying_face:


missing stats

Hi,
Maybe this is a dumb question but I didn't see any output file or statistics after running the example yaml. I installed ramulator in Linux and ran the example config in the standalone mode. Am I missing something? Thnaks
Ly

Hi Ly,
The stats are printed to stdout. We organize stats hierarchically in the form of YAML documents. It is possible to make the stats print to a file. We will likely include this functionality the next time we update this public repo.
Hope this helps!


objectDefintions update

first start of the adapter changed because of error messages empty object
open-epaper-link.0 | 2023-11-28 09:15:49.850 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.850 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.849 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.849 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.849 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.849 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.849 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.848 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.848 | error | empty object! open-epaper-link.0 | 2023-11-28 09:15:49.847 | error | empty object!

changed delete description

thank you, wills solve the ESLint formatting parts after merging


Improve visual appearance of the new "subscriber counts" system

Following up on #3989 / #7464, there's a couple things we definitely want to do here:

We should add a similar hover behavior on the subscriber count that says "Subscriber count" to the "Average weekly traffic" text.
We need to dosomething reasonable for alignment (I think ideally all the person icons and all the chat messages icons would be vertically aligned, if we can make that work and look good).  See https://chat.zulip.org/#narrow/stream/101-design/subject/stream.20traffic.20data/near/525256 for some discussion.  Also we might want to use a traffic sign icon rather than the "chat symbols" one to depict this.

Feedback on how exactly this should work is very welcome.
(Tagging this as a release blocker since I want us to make explicit decisions on how this appears before the 1.8 release).

I would like to try this out.
@zulipbot claim

@nkmishra1997 cool; when are you likely to have something for this?  I'd like to be sure we get this fixed in the next couple days for the release, so it's possible it should be handed off to @YJDave.

@timabbott I have tried stacking the data vertically. See if it works for this. I am available to work on this for the next couple of days, however, if needed, @YJDave can definitely take it up :+1:

sorry for the late reply, I was busy in hackathons. I will look into #8718


An SSL error has occurred and a secure connection to the server cannot be made

Install failed: Download failed with error: Error Domain=NSURLErrorDomain Code=-1200 "An SSL error has occurred and a secure connection to the server cannot be made." UserInfo={NSErrorFailingURLStringKey=https://github.com/Sunnyyoung/WeChatTweak-macOS/releases/latest/download/WeChatTweak.framework.zip, NSLocalizedRecoverySuggestion=Would you like to connect to the server anyway?, _kCFStreamErrorDomainKey=3, _NSURLErrorFailingURLSessionTaskErrorKey=LocalDownloadTask <401F6CA5-13E7-410A-B70A-B92D9468DD6C>.<1>, _NSURLErrorRelatedURLSessionTaskErrorKey=(
"LocalDownloadTask <401F6CA5-13E7-410A-B70A-B92D9468DD6C>.<1>"
), NSLocalizedDescription=An SSL error has occurred and a secure connection to the server cannot be made., NSErrorFailingURLKey=https://github.com/Sunnyyoung/WeChatTweak-macOS/releases/latest/download/WeChatTweak.framework.zip, NSUnderlyingError=0x600000700f00 {Error Domain=kCFErrorDomainCFNetwork Code=-1200 "(null)" UserInfo={_kCFStreamPropertySSLClientCertificateState=0, _kCFNetworkCFStreamSSLErrorOriginalValue=-9836, _kCFStreamErrorDomainKey=3, _kCFStreamErrorCodeKey=-9836, _NSURLErrorNWPathKey=satisfied (Path is satisfied), viable, interface: lo0}}, _kCFStreamErrorCodeKey=-9836}

网络问题

我也是安装失败，跟这个报错一样

手动make就好了

手动make就好了

咋手动make

手动make就好了

咋手动make

我用这个示例手动make就可以了 用cli工具也是这个报错
手动make安装


Can I change z-index property of the labels?

Hey all,
I have multiple average lines on the chart and when they are close to each other I want the labels on top of the lines not behind them. is there an option to control this behavior ?

Same !

This looks like a duplicate of #94; see #161.

Closing as duplicate


[Ansible Installer] Istio Demo file is not found in Ansible Installer on Istio 1.0.0

Description
Istio Demo file is not found in Ansible Installer.
Executed the following command ansible-playbook main.yml -e '{"istio": {"release_tag_name": "1.0.0", "auth": false, "delete_resources": true}}' on release-1.0` branch
Expected Behaviour

Istio installed

Actual Behaviour

TASK [istio : Copy definition file] ************************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"changed": true, "cmd": ["cp", "/home/user/istio/install/kubernetes/istio-demo.yaml", "/tmp/ansible.e8nrr0lJ0v/def.yml"], "delta": "0:00:00.002999", "end": "2018-08-06 18:02:07.137754", "msg": "non-zero return code", "rc": 1, "start": "2018-08-06 18:02:07.134755", "stderr": "cp: cannot stat '/home/user/istio/install/kubernetes/istio-demo.yaml': No such file or directory", "stderr_lines": ["cp: cannot stat '/home/user/istio/install/kubernetes/istio-demo.yaml': No such file or directory"], "stdout": "", "stdout_lines": []}

cc @sdake @gyliu513  @geoand @ymesika

This looks like having a typo: /home/usert/istio/install/kubernetes/istio-demo.yaml
usert?

That indeed looks like a type, it's weird however that such a type shows up...

@ymesika you are right.
There was typo on my error. Updated!

@gbaufake I probably won't be able to look at this for a couple days :(

I think I have figured out.
By downloading the release and executing the ansible command works like a charm.
Since the istio-demo.yaml is included  https://github.com/istio/istio/blob/f3a6d521a1a3225ac855b95fada056e234dbf503/.gitignore#L39.]
When you do clone the repository, the ansible repository will not work since it doesn't contain the istio-demo file (which I don't know if it is on purpose or not).
Anyhow,
Thanks for attention.


[benchmark] Benchmark broken since #125

See https://github.com/MystenLabs/narwhal/commit/767a364e9937d38e52db1df406aa9bfc3bb4d1eb for the benchmark failure, and CI benchmark success on the prior commit.
This is actually a bootstrap failure on the config file, which does not include the new parameters:
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1
Error: Failed to load the node's parameters

Caused by:
    Failed to read config file '.parameters.json': missing field `block_synchronizer` at line 9 column 1

This is due to the deserialization of the new block synchronizer parameters being absent of the parameters.json created in benchmarks from this entry point
Once this is fixed (by unplugging the parameters):
huitseeker@dapple.local➜narwhal/benchmark/logs(repair_benchmarks)» grep panick *                                                                                [15:39:56]
primary-0.log:thread 'tokio-runtime-worker' panicked at 'all branches are disabled and there is no else branch', /Users/huitseeker/tmp/narwhal/primary/src/block_synchronizer/mod.rs:221:13
primary-1.log:thread 'tokio-runtime-worker' panicked at 'all branches are disabled and there is no else branch', /Users/huitseeker/tmp/narwhal/primary/src/block_synchronizer/mod.rs:221:13
primary-2.log:thread 'tokio-runtime-worker' panicked at 'all branches are disabled and there is no else branch', /Users/huitseeker/tmp/narwhal/primary/src/block_synchronizer/mod.rs:221:13
primary-3.log:thread 'tokio-runtime-worker' panicked at 'all branches are disabled and there is no else branch', /Users/huitseeker/tmp/narwhal/primary/src/block_synchronizer/mod.rs:221:13

This is due to the BlockSynchronizer being spawned in the primary with inactive channels, which .recv() case cannot be true and gets instantly disabled.

Benchmark is hot-fixed by #176. The real fixes will probably be part of subsequent PRs.

#173 is in a good place to start adding stuff to the fabfile for config, but is perhaps slightly off-topic,
#127 has a good chance of fixing most issues, including the fabfile as well as plugging in real channels in the BlockSynchronizer.


Update cortex-m to 0.7.1

r? @adamgreig
(rust-highfive has picked a reviewer for you, use r? to override)

r? @adamgreig
(rust-highfive has picked a reviewer for you, use r? to override)


Terraspace show --json adds newlines to json output

I use the command: TS_ENV=test terraspace show aks --plan aks.tfplan --json to produce a json file which I then parse with jq to show some statistics about the changes to be made in a merge request. However terraspace show --json adds several newlines to (it seems) random places in this json file. I tried to do the same in the cache/stack dir with terraform show and it does not add any newlines to random places.
Terraspace:
TS_ENV=test terraspace init aks && TS_ENV=test terraspace plan aks --out aks.tfplan && TS_ENV=test terraspace show aks --plan aks.tfplan --json | wc -l

3

Terraform (in the cache/stack directory):
terraform show -json aks.tfplan | wc -l

1

I only expect one newline at the end of the file. Terraspace: 0.6.11, Terraform: 1.0.2

you can create temporary file as json like below
TS_ENV=test terraspace output aks --format=json -o test.json

you can create temporary file as json like below
TS_ENV=test terraspace output aks --format=json -o test.json

This doesn't produce anything for me.

you can create temporary file as json like below
TS_ENV=test terraspace output aks --format=json -o test.json

This doesn't produce anything for me:
❯ TS_ENV=test terraspace output aks --format=json
Current directory: .terraspace-cache/westeurope/test/stacks/aks
=> terraform output -json
{}

I guess this works after something is created.

yes, this should be running after terraform up

Fix released in 0.6.15

you can create temporary file as json like below TS_ENV=test terraspace output aks --format=json -o test.json

@drbjim How would one find out which arguments are supported here (e.g. that the format parameter is supported)? I did not find it in the CLI help or the documentation. Is there a specific place in the code you could point to where this is handled?


Question about performant use of IORING_REGISTER_BUFFERS

Is there any performance issue or synchronization that would take place if multiple IORING_OP_READ_FIXED / IORING_OP_WRITE_FIXED calls use the same buffer index in IORING_REGISTER_BUFFERS assuming the address ranges ([.addr, addr + len]) never overlap (assume offset is always 0 as well).
What I am considering doing is implementing a lightweight malloc for the read / write buffers using a single 1GB register buffer. (This seems like it would be lighter weight that IORING_OP_PROVIDE_BUFFERS and IORING_BUFFER_SELECT).
I took a look at io_import_fixed and I don't see any mechanism for locking buffers by buf_index but I would appreciate if anyone who knew the code base could verify / correct my understanding.
Thank you!

Is there any performance issue or synchronization that would take place if multiple IORING_OP_READ_FIXED / IORING_OP_WRITE_FIXED calls use the same buffer index in IORING_REGISTER_BUFFERS assuming the address ranges ([.addr, addr + len]) never overlap (assume offset is always 0 as well).

There is no additional overhead for that. Note that the kernel manages buffers but not what's in them, so if user issues concurrent IO to overlaping memory chunks, it may get mixed/etc. data.

What I am considering doing is implementing a lightweight malloc for the read / write buffers using a single 1GB register buffer. (This seems like it would be lighter weight that IORING_OP_PROVIDE_BUFFERS and IORING_BUFFER_SELECT).

fwiw, registered buffers are great because memory is pinned and prepared, so if underlaying file may benefit from it, it will. E.g. direct IO will save a lot, but IIRC buffered would not do much better.

I took a look at io_import_fixed and I don't see any mechanism for locking buffers by buf_index but I would appreciate if anyone who knew the code base could verify / correct my understanding.

Not sure why would you even lock by index. Registration and unregistration are currently implemented as full quiesce of the ring.

but IIRC buffered would not do much better.

registered buffers or provided buffers?
Also as a side note are you working on fd == -2 -> skip update? If not I thought I'd give it a shot as it seems simple enough a change. I have 0 experience with linux contributions so if you are already working on it probably best for you to do the patch.

This patch makes it so that specify a file descriptor value of -2 will
skip updating the corresponding fixed file index.
This will allow for users to reduce the number of syscalls necessary
to update a sparse file range when using the fixed file option.

Answering the github thread -- it's indeed a simple change, I had it the
same day you posted the issue. See below it's a bit cleaner. However, I
want to first review "io_uring: buffer registration enhancements", and
if it's good, for easier merging/etc I'd rather prefer to let it go
first (even if partially).

Noah, want to give it a try?

Hi @isilence I would love to give it a shot (I will reply as such to kernel thread). In the interest of not polluting the development thread with too many questions I'm asking a few here.

I've just sent a prep patch

I am sure what you mean by prep patch. Do you mean the patch "[PATCH 1/1] io_uring: cleanup files_update looping"?  Thats the only recent patch I see from you that touches __io_sqe_files_update

with it you can implement it cleaner with one continue.

Got it.

This patch makes it so that specify a file descriptor value of -2 will
skip updating the corresponding fixed file index.
This will allow for users to reduce the number of syscalls necessary
to update a sparse file range when using the fixed file option.

Answering the github thread -- it's indeed a simple change, I had it the
same day you posted the issue. See below it's a bit cleaner. However, I
want to first review "io_uring: buffer registration enhancements", and
if it's good, for easier merging/etc I'd rather prefer to let it go
first (even if partially).

Noah, want to give it a try?

Hi @isilence I would love to give it a shot (I will reply as such to kernel thread). In the interest of not polluting the development thread with too many questions I'm asking a few here.

I've just sent a prep patch

I am sure what you mean by prep patch. Do you mean the patch "[PATCH 1/1] io_uring: cleanup files_update looping"?  Thats the only recent patch I see from you that touches __io_sqe_files_update

with it you can implement it cleaner with one continue.

Got it.

Hi @isilence I would love to give it a shot (I will reply as such to kernel thread). In the interest of not polluting the development thread with too many questions I'm asking a few here.

Consistency is better, those who are not interested can easily skip it, but torn threads bouncing between email and github would give hard time retrospectively collecting these pieces.

I've just sent a prep patch

I am sure what you mean by prep patch. Do you mean the patch "[PATCH 1/1] io_uring: cleanup files_update looping"? Thats the only recent patch I see from you that touches __io_sqe_files_update

Right this one. Jens took it but haven't yet pushed into a repo. See 5.12 dev branch
https://git.kernel.dk/cgit/linux-block/log/?h=for-5.12/io_uring
It should appear there later, but you can always apply it yourself to not waste time

Hi @isilence I would love to give it a shot (I will reply as such to kernel thread). In the interest of not polluting the development thread with too many questions I'm asking a few here.

Consistency is better, those who are not interested can easily skip it, but torn threads bouncing between email and github would give hard time retrospectively collecting these pieces.

I've just sent a prep patch

I am sure what you mean by prep patch. Do you mean the patch "[PATCH 1/1] io_uring: cleanup files_update looping"? Thats the only recent patch I see from you that touches __io_sqe_files_update

Right this one. Jens took it but haven't yet pushed into a repo. See 5.12 dev branch
https://git.kernel.dk/cgit/linux-block/log/?h=for-5.12/io_uring
It should appear there later, but you can always apply it yourself to not waste time

@goldsteinn, was it answered? It can be closed, I guess


Tab to select

Overview
Adds a setting tabToSelect to allow tabbing to select suggestion.
Closes https://github.com/moroshko/react-autosuggest/issues/163
Details

Adds property tabToSelect (defaults to false)
Sends Tab event type to onSuggestionsUpdateRequested and onSuggestionSelected
Updates README documentation with setting and event types for above functions
Adds tests for both true and false values of the setting

@rickhanlonii Thanks a lot for a detailed PR!
I must admit that I'm a bit confused with the functionality you are trying to achieve. In #163 you mentioned "Slack user mentions". As far as I can see, pressing Tab in Slack is somewhat equivalent to pressing Down, not pressing Enter, i.e. it moves to the next suggestion rather than selecting the focused one. The URL bar in Chrome behaves the same. Am I missing something?

Great point @moroshko, I'll update and re-open.


Homepage newsletter

LGTM the only things I'm not sure about are these different paddings in email address - subscribe button line, maybe we can make it more even?

:christmas_tree:


rl-2311 updates

Description of changes
starting with some low hanging fruit editorial updates
Things done

Built on platform(s)

[ ] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
23.11 Release Notes (or backporting 23.05 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[ ] Fits CONTRIBUTING.md.

As this PR got its first conflict, I'll use my time at the oceansprint to work on splitting the release notes and provide tools to autogenerate the combined markdown file.


Deal with special characters like German umlaut etc.

Hi!
Really like your completion plugins a lot, thanks for sharing your great results with the community!
I just identified that the buffer source cannot deal with special (for Germans not so special) characters.
The cmp-nvim-lsp source is not having that issue.
It is quite simple to demonstrate that with some screenshots:

Background: https://en.wikipedia.org/wiki/%C3%9C

Background: https://en.wikipedia.org/wiki/%C3%9F
Here, cmp-nvim-lsp handles the character ß correctly and cmp-buffer doesn't:

Here, cmp-nvim-lsp handles the character Ä correctly and cmp-buffer doesn't:

All best and greeting from Berlin,
Jamil

I am also having this problem using Swedish åäö, which are not recognised as a part of the world.

Try
...
{ name = 'buffer',
  option = {
    keyword_pattern = [[\K\k*]],
  }
},
...

That solves the issue, thanks!


When upgrading an SPFx project, is it an idea to have --output md > somefile.md as the default behaviour?

It might just be me, but when updating an SPFx solution using the CLI, I always output the report as an md file, and I always scroll to the summary to get the aggregated pieces of what I need to do.
So for me, having a default ouput as a file named something like upgrade-1.4.0-1.5.1.md, and also having the summary at the top and not at the bottom of the report would save me a few keystrokes and seconds :)

The first change we could do easily. I'm curious about the second part as currently the text output is kind of useless with the resolutions which you need to do the upgrade. Thinking if the text mode shouldn't be merely a listing of the issues without any resolutions, but would that be helpful at all?

This is how I'd like to see the text output.
Console command
---------------------
npm i<EMAIL_ADDRESS>--save-exact
npm i<EMAIL_ADDRESS>--save-exact
npm i<EMAIL_ADDRESS>--save-exact
npm i<EMAIL_ADDRESS>--save-exact
npm i<EMAIL_ADDRESS>--save-exact
npm i<EMAIL_ADDRESS>--save-exact
npm i<EMAIL_ADDRESS>--save-exact
npm i<EMAIL_ADDRESS>--save-exact -D
npm i<EMAIL_ADDRESS>--save-exact -D
npm i<EMAIL_ADDRESS>--save-exact -D
npm i<EMAIL_ADDRESS>--save-exact -D

.yo-rc-json
------------
{
  "@microsoft/generator-sharepoint": {
    "version": "1.6.0"
  }
}

etc.

So for the text output, you'd like resolutions grouped per file/type without additional explanation?

As a summary yes. The why is not important to me, I need to upgrade :-)

Thinking about it, the challenge with showing only the file name and all resolutions grouped per file is that it would be impossible for you to tell the difference between adding something to the file and removing something from it. Currently the resolution shows the affected code but whether you should add that to the particular file or remove from it is included in the finding's description. So to make it clearer, what we could do is the following:
Console command
---------------------
npm i<EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS>-SE
npm i<EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS>-DE

.yo-rc-json
------------
Update version in .yo-rc.json:
{
  "@microsoft/generator-sharepoint": {
    "version": "1.6.0"
  }
}

Remove framework in .yo-rc.json:
{
  "@microsoft/generator-sharepoint": {
    "framework": "react"
  }
}

What do you think @wobba @sympmarc?


Fix AMA swagger for completeness. R3007

ARM API Information (Control Plane)
MSFT employees can try out our new experience at OpenAPI Hub  - one location for using our validation tools and finding your workflow.

Azure 1st Party Service can try out the Shift Left experience to initiate API design review from ADO code repo.  If you are interested, may request engineering support by filling in with the form https://aka.ms/ShiftLeftSupportForm.
Changelog
Add a changelog entry for this PR by answering the following questions:

What's the purpose of the update?

[ ] new service onboarding
[ ] new API version
[ ] update existing version for new feature
[x] update existing version to fix swagger quality issue in s360
[ ] Other, please clarify

When are you targeting to deploy the new service/feature to public regions? Please provide the date or, if the date is not yet available, the month.
When do you expect to publish the swagger? Please provide date or, the the date is not yet available, the month.
By default, Azure SDKs of all languages (.NET/Python/Java/JavaScript for both management-plane SDK and data-plane SDK, Go for management-plane SDK only ) MUST be refreshed with/after swagger of new version is published. If you prefer NOT to refresh any specific SDK language upon swagger updates in the current PR, please leave details with justification here.

Contribution checklist (MS Employees Only):

[x] I commit to follow the Breaking Change Policy of "no breaking changes"
[x] I have reviewed the documentation for the workflow.
[x] Validation tools were run on swagger spec(s) and errors have all been fixed in this PR. How to fix?

If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist

Applicability: :warning:
If your changes encompass only the following scenarios, you should SKIP this section, as these scenarios do not require ARM review.

Change to data plane APIs
Adding new properties
All removals

Otherwise your PR may be subject to ARM review requirements. Complete the following:

[ ] Check this box if any of the following apply to the PR so that the label "ARMReview" and "WaitForARMFeedback" will be added by bot to kick off ARM API Review.  Missing to check this box in the following scenario may result in delays to the ARM manifest review and deployment.

Adding a new service
Adding new API(s)
Adding a new API version
-[ ] To review changes efficiently, ensure you copy the existing version into the new directory structure for first commit and then push new changes, including version updates, in separate commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki.

[ ] Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.

[ ] If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.

Breaking Change Review Checklist
If you have any breaking changes as defined in the Breaking Change Policy, request approval from the Breaking Change Review Board.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Additional details on the process and office hours are on the Breaking Change Wiki.
NOTE: To update API(s) in public preview for over 1 year (refer to Retirement of Previews)
Please follow the link to find more details on PR review process.

This fix is being made for following S360 for swagger completeness.
https://portal.azure-devex-tools.com/amekpis/linting/detail?errorId=721D62A4-4696-4718-A8E1-3CC05D1AAA7A

@leni-msft Can you please review this PR. This is for fixing S360 issues.

@jianyexi please help reivew this PR fixing Lint R3007 PutGetPatchResponseSchema

@vayada please request breaking change approval following Action in https://github.com/Azure/azure-rest-api-specs/pull/20667#issuecomment-1244931406 . and fix the Lint(RPaaS) error

@tadelesh please help check go sdk breaking changes

@vayada please request breaking change approval following Action in #20667 (comment) . and fix the Lint(RPaaS) error

@leni-msft  RPaaS Linter errors are false alarm as agreed with RPaaS team. As you can see this is not coming from current change. We have exception for this since starting.
I will request Breaking Change Approval for this.

@tadelesh please help check go sdk breaking changes

Will review after breaking change arch review.

@tadelesh Breaking Change ARCH review wants you to review first. So, can you please do that.

@tadelesh Breaking Change ARCH review wants you to review first. So, can you please do that.

The Go breaking is from this old PR. @leni-msft I don't know how to handle the swagger breaking from this PR, could you help?

@josefree Could you give guidance here? This PR is to fix S360 Lint error, and may cause breaking changes to both API and SDK.

Approved for Go breaking as it is caused by this PR.

@vayada Go SDK breaking changes have been approved, please request BC board review


feat: Design and implement monitoring and alters for price discrepancies across exchanges[DEV-1300]

This PR creates /arbitrage endpoint that returns current price of CHEQ token across different markets and returns a possible arbitrage opportunities. It uses our monitor market api which is a serverless digital ocean function that uses coin gecko api to get latest price for CHEQ token. monitor market api does the logic for calculating possible arbitrage opportunities and returns data to be consumed directly by our cloud-flare worker.
It does also triggers a Webhook when the market arbitrage threshold is greater or equals to 1. @ankurdotb time to setup the zap with webhook URL.

@ankurdotb now using deployed version of market-monitoring-api


Component Browser Alias Behaviour Not Working

The component browser isn't quite working correctly with component aliases.
Display alias when searching by alias
When I search using an alias I expect that alias to show in the component browser with the actual component name in brackets afterwards. e.g. For component Take and alias First
Expected:

Actual:

Multiple aliases should work
e.g. For component Take and alias Last
Expected:

Actual:

Searching by alias initials should work
e.g. Searching for gb should find aggregate via the group_by alias
Expected:

Actual:

### Tasks
- [ ] https://github.com/enso-org/enso/issues/9148
- [ ] Multiple aliases should work
- [ ] Searching by alias initials should work

oops... didn't notice kaz was assigned

oops... didn't notice kaz was assigned

No problem, I hadn't started on parts 2-3 yet anyway


fkdl0048's mentoring meeting 7, 2022-12-26

오후 11시
google meet

https://github.com/fkdl0048/Mentoring-Project/discussions/33


What could be the reason for Unknown error format?

Exponent::Push::UnknownError: Unknown error format: {}
  from exponent-server-sdk (0.1.0) lib/exponent-server-sdk.rb:143:in `handle'
  from exponent-server-sdk (0.1.0) lib/exponent-server-sdk.rb:51:in `publish'
  from app/workers/expo_worker.rb:15:in `perform'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:196:in `execute_job'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:164:in `block (2 levels) in process'
  from sidekiq (6.1.2) lib/sidekiq/middleware/chain.rb:138:in `block in invoke'
  from newrelic_rpm (6.14.0) lib/new_relic/agent/instrumentation/sidekiq.rb:35:in `block in call'
  from newrelic_rpm (6.14.0) lib/new_relic/agent/instrumentation/controller_instrumentation.rb:376:in `perform_action_with_newrelic_trace'
  from newrelic_rpm (6.14.0) lib/new_relic/agent/instrumentation/sidekiq.rb:30:in `call'
  from sidekiq (6.1.2) lib/sidekiq/middleware/chain.rb:140:in `block in invoke'
  from sentry-raven (3.1.1) lib/raven/integrations/sidekiq/cleanup_middleware.rb:7:in `call'
  from sidekiq (6.1.2) lib/sidekiq/middleware/chain.rb:140:in `block in invoke'
  from sidekiq (6.1.2) lib/sidekiq/middleware/chain.rb:143:in `invoke'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:163:in `block in process'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:136:in `block (6 levels) in dispatch'
  from sidekiq (6.1.2) lib/sidekiq/job_retry.rb:111:in `local'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:135:in `block (5 levels) in dispatch'
  from sidekiq (6.1.2) lib/sidekiq/rails.rb:14:in `block in call'
  from activesupport (<IP_ADDRESS>) lib/active_support/execution_wrapper.rb:88:in `wrap'
  from activesupport (<IP_ADDRESS>) lib/active_support/reloader.rb:72:in `block in wrap'
  from activesupport (<IP_ADDRESS>) lib/active_support/execution_wrapper.rb:88:in `wrap'
  from activesupport (<IP_ADDRESS>) lib/active_support/reloader.rb:71:in `wrap'
  from sidekiq (6.1.2) lib/sidekiq/rails.rb:13:in `call'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:131:in `block (4 levels) in dispatch'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:257:in `stats'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:126:in `block (3 levels) in dispatch'
  from sidekiq (6.1.2) lib/sidekiq/job_logger.rb:13:in `call'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:125:in `block (2 levels) in dispatch'
  from sidekiq (6.1.2) lib/sidekiq/job_retry.rb:78:in `global'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:124:in `block in dispatch'
  from sidekiq (6.1.2) lib/sidekiq/logger.rb:10:in `with'
  from sidekiq (6.1.2) lib/sidekiq/job_logger.rb:33:in `prepare'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:123:in `dispatch'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:162:in `process'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:78:in `process_one'
  from sidekiq (6.1.2) lib/sidekiq/processor.rb:68:in `run'
  from sidekiq (6.1.2) lib/sidekiq/util.rb:15:in `watchdog'
  from sidekiq (6.1.2) lib/sidekiq/util.rb:24:in `block in safe_thread'

You're probably sending more than 100 devices per request. Or you're sending multiple expo-experiences inside token array.
I had exactly these two issues.


Recognise shebangs

Syntax highlighting seems to br triggered by file types, i.e. file extensions. So that fred.py is highligghted as Python syntax. Unfortunately this means that a file called just fred, which is executable and has #! /usr/bin/env python on the first line is not recognised, hence not highlighted.

Oh this is a good idea. I'm using the rich.syntax.Syntax.guess_lexer method from Rich to handle this currently since the Syntax class is handling almost everything else. In turn it uses pygments to handle it.
https://rich.readthedocs.io/en/stable/reference/syntax.html#rich.syntax.Syntax.guess_lexer
https://github.com/juftin/browsr/blob/26a1bf3488916f3c26f96d40c8f33eabee68d358/browsr/browsr.py#L175-L184
I wonder if checking for a shebang would be a welcome addition to the upstream libraries too.


Additional Features

Add an option to votekick exploiters, disabled by default preferably. most consider exploiting to be as bad as cheating, and some exploits are worse then cheating due to the sheer disruption caused.
Add an option to disable message prints for racism, and print suspicious/exploitive players.
Add an option to disable the scoreboard marks for racism/suspicious

Add an option to disable the scoreboard marks for racism/suspicious

like the shining thing? Just change the color to completely transparent


Add arguments to disable loading keybindings or completion

NixOS switched (https://github.com/NixOS/nixpkgs/pull/298692) to the new way of loading the completion files which was added in #3675 but that removes support for not loading the completion, so I just added options for that.

Thanks, but I'm going to keep it simple and not add more options. As noted in the instruction, you can still load individual script files if you need finer control.
https://github.com/junegunn/fzf?tab=readme-ov-file#setting-up-shell-integration

If you have an older version of fzf, or want finer control, you can source individual script files in the /shell directory.


Teleport RBAC

We can't remember what this is

@gawertm do you remember what this was about?

this was about introducing different roles with different viewing permissions e.g. of audit logs and session recordings. It came basically from the fact that with Teleport session recording, we have recordings of what a GS employee exactly types. After clarifying with team-team we only restricted the permissions for everyone to see only their own logs/recordings. so we can close this one IMO


Premium storage caching on B-series VMs

Hello,
The documentation regarding Premium storage caching on B-series VMs seems to be outdated/wrong.
It's clearly indicated that it is not supported yet it is enabled by default on the OS disk at the VM's creation.
Furthermore, it's possible to enable Read/Write cache on Premium data disks attached to a B-series VM via the following procedure:

Create a B2s VM
Add a premium data disk. By default, its cache setting is set to None
Resize the B2s VM to a D2s_v3 VM
Change the cache setting on the newly added premium data disk to Read/Write
Resize the D2s_v3 VM to a B2s VM
The newly added premium data disk retain its updated cache setting after the resize

From what I can see, Premium storage caching IS supported on B-series VMs:

It's supported out of the box on the OS disk
It's supported via a workaround on data disks

Thanks,
FrenezOrg

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 0cd2e24b-0fd3-d698-2840-3f8206933026
Version Independent ID: 832dc371-480a-c74a-e956-e1904473af31
Content: B-series burstable - Azure Virtual Machines - Azure Virtual Machines
Content Source: articles/virtual-machines/sizes-b-series-burstable.md
Service: virtual-machines
Sub-service: sizes
GitHub Login: @ayshakeen
Microsoft Alias: ayshak

Thanks for the feedback! I have assigned the issue to the content author to investigate further and update the document as appropriate.

Hello,
Any news on this?
Thanks,
FrenezOrg

@ayshakeen Did you get a chance to look into this doc-enhancement request? Thanks :)

Hello,
Any news on this?
Thanks,
FrenezOrg

I checked with the team and they stated it is not supported. You might have found a work around however out of the box, it is not supported.
#please-close

@mimckitt sorry for digging out old ticket, but I could not find an answer. Could you clarify the following please?
a). B-series VM OS disk caching option preselected to default Read/write after provisioning.
But if no caching is supported on B-series VMs what is it in reality then? None or still Read only?

b). How could you explain VM Cached Bandwidth Consumed Percentage and VM Cached IOPS Consumed Percentage still showing spikes in Metrics? I would expect these to be always zero if no caching is supported on B-series VMs.


Migrate Reactr SDKs to E2 Core

This PR as is migrates TinyGo's SDKs with deprecated functionality removed.
The test plugins in sat/engine/testdata/tinygo-* can be built individually with this command (if you have TinyGo installed):
tinygo build -o $(basename $(pwd)).wasm -target wasi .

Stale, see sdk repo.


java.lang.IllegalStateException: Intent filter for: BrowserTabActivity is missing.  Please make sure you have the following activity in your AndroidManifest.xml

Hi everybody. I'm trying to use msal_mobile, but when i launch the app in debug mode i've got this issue:
FATAL EXCEPTION: main E/AndroidRuntime(17653): Process: com.example.microsoft_handle, PID: 17653 E/AndroidRuntime(17653): java.lang.IllegalStateException: Intent filter for: BrowserTabActivity is missing.  Please make sure you have the following activity in your AndroidManifest.xml E/AndroidRuntime(17653): E/AndroidRuntime(17653): <activity android:name="com.microsoft.identity.client.BrowserTabActivity"> E/AndroidRuntime(17653): 	<intent-filter> E/AndroidRuntime(17653): 		<action android:name="android.intent.action.VIEW" /> E/AndroidRuntime(17653): 		<category android:name="android.intent.category.DEFAULT" /> E/AndroidRuntime(17653): 		<category android:name="android.intent.category.BROWSABLE" /> E/AndroidRuntime(17653): 		<data E/AndroidRuntime(17653): 			android:host="com.example.XXXXXXXXXXXXXX" E/AndroidRuntime(17653): 			android:path="/5XXXXXXXXXXXXXXXXXXX" E/AndroidRuntime(17653): 			android:scheme="msauth" /> E/AndroidRuntime(17653): 	</intent-filter> E/AndroidRuntime(17653): </activity>
my android manifest:
<manifest xmlns:android="http://schemas.android.com/apk/res/android"     package="com.example.microsoft_handle">     <!-- io.flutter.app.FlutterApplication is an android.app.Application that          calls FlutterMain.startInitialization(this); in its onCreate method.          In most cases you can leave this as-is, but you if you want to provide          additional functionality it is fine to subclass or reimplement          FlutterApplication and put your custom class here. -->     <uses-permission android:name="android.permission.INTERNET" />     <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />     <application         android:name="io.flutter.app.FlutterApplication"         android:label="microsoft_handle"         android:icon="@mipmap/ic_launcher">         <activity             android:name=".MainActivity"             android:launchMode="singleTop"             android:theme="@style/LaunchTheme"             android:configChanges="orientation|keyboardHidden|keyboard|screenSize|smallestScreenSize|locale|layoutDirection|fontScale|screenLayout|density|uiMode"             android:hardwareAccelerated="true"             android:windowSoftInputMode="adjustResize">             <intent-filter>                 <action android:name="android.intent.action.MAIN"/>                 <category android:name="android.intent.category.LAUNCHER"/>             </intent-filter>         </activity>         <activity android:name="com.microsoft.identity.client.BrowserTabActivity">             <intent-filter>                 <action android:name="android.intent.action.VIEW" />                 <category android:name="android.intent.category.DEFAULT" />                 <category android:name="android.intent.category.BROWSABLE" />               <data                     android:host="com.example.XXXXXXXXX"                     android:path="/5XXXXXXXXXXXXXXXXXXX"                     android:scheme="msauth" />             </intent-filter>         </activity>         <!-- Don't delete the meta-data below.              This is used by the Flutter tool to generate GeneratedPluginRegistrant.java -->         <meta-data             android:name="flutterEmbedding"             android:value="2" />     </application> </manifest>
I don't know where's the problem. Anybody can help me??
Thank you

You seem to be doing exactly as instructed. Have you tried invalidating the cache under the "File" tab?

Thank u but I solved by switching library
Il Lun 10 Ago 2020, 12:43 jlieow<EMAIL_ADDRESS>ha scritto:

You seem to be doing exactly as instructed. Have you tried invalidating
the cache under the "File" tab?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/gbwisx/msal_mobile/issues/4#issuecomment-671283793,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADEWB4IB4JEUU2L6FY24M3LR77FOPANCNFSM4L4J2YWA
.

You switched to which library? you can share your implementation if you dont mind on git

Thank u but I solved by switching library Il Lun 10 Ago 2020, 12:43 jlieow<EMAIL_ADDRESS>ha scritto:
…
You seem to be doing exactly as instructed. Have you tried invalidating the cache under the "File" tab? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADEWB4IB4JEUU2L6FY24M3LR77FOPANCNFSM4L4J2YWA .

I'm facing same problem, may i know which library you have switched to ?


Add typescript definitions

what?
This adds typescript definition for the library. Not complete, but all series types should have types.
References highcharts types.
Somewhat related to discussion in #113 and in https://github.com/whawker/react-jsx-highcharts/issues/267#issuecomment-578111704
why?

Better IDE experience even when writing plain js.
Type checking for those who use typescript

caveats

Event handlers are not typed, instead the evented components accept any props in addition to highcharts options

I am no way expert in writing definition files. I suggest that we release this and see what happens? It's easy to remove if it causes too much a headache. Maybe adding something in the release notes about "beta types".

Codecov Report

Merging #303 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #303   +/-   ##
=======================================
  Coverage   88.17%   88.17%
=======================================
  Files          82       82
  Lines        1049     1049
  Branches      206      206
=======================================
  Hits          925      925
  Misses        108      108
  Partials       16       16

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df21107...7b862b9. Read the comment docs.

We've been maintaining our own typings for (a subset of) RJH, so this is really great!
You haven't considered moving the codebase towards typescript? I realise it's quite some work to get there, but I think it would be worth while. If not, please make sure to keep the typings up-to-date. Misleading types are worse than no types.

We've been maintaining our own typings for (a subset of) RJH, so this is really great!

Can you please paste this definition to your module declaration and report if it works for you?

You haven't considered moving the codebase towards typescript? I realise it's quite some work to get there, but I think it would be worth while.

That's up to whawker to decide. Previously typescript was quite cumbersome to use with react, but latest releases have been better. It's however quite an effort and adds complexity.

Can you please paste this definition to your module declaration and report if it works for you?

We're using the react-jsx-highstock package and the highstock specific components are missing, but other than that it seems to work. There are a few any types though, which could be improved on further.

There are a few any types though, which could be improved on further.

Do you happen to have definitions for those?

Well, I would have to go through the components and look at the code. One simple example is Credits where the props type should be {enabled: boolean}.

Well, I would have to go through the components and look at the code. One simple example is Credits where the props type should be Highcharts.CreditsOptions & { enabled?: boolean }.

I added type for <Credits /> in 19a4c688e9ee87a0b9852bebcab3c53e769deef6

Nice @anajavi, this is looking good! I would like to have the event handlers listed out though, instead of [x: string]: any, so they also get their appropriate types.

Nice @anajavi, this is looking good! I would like to have the event handlers listed out though, instead of [x: string]: any, so they also get their appropriate types.

Something like this should probably work:
  type ChartProps = {
    onSelection?: Highcharts.ChartSelectionCallbackFunction;
    [x: string]: any; // TODO: this is here to allow eventhandlers like onAfterAddSeries
  } & Partial<Highcharts.ChartOptions>;

  export function Chart(props: ChartProps): ReactElement;

There are however more events in highcharts than there are in their typings. One example is afterAddSeries.

Nice @anajavi, this is looking good! I would like to have the event handlers listed out though, instead of [x: string]: any, so they also get their appropriate types.

@veloek I added the event handler typings if you like to try.

You haven't considered moving the codebase towards typescript?

To be honest I don't know Typescript, so I would struggle to maintain the project. It is something I intend to learn but I don't use it day-to-day currently, like I do with JS

I suggest that we release this and see what happens?

Worth releasing as a beta first?

Worth releasing as a beta first?

This certainly won't break anybodys application, but might confuse a bit. Maybe a beta release and mention in the #259 might get some feedback?
I intend to add some kind of test or eslint configuration for this later so we see if new highcharts release breaks the typings(most likely the eventhandlers).

Apologies for the insane delay on this, life during Covid just ends up trying to keep way too many balls in the air at once.
I've published a beta<EMAIL_ADDRESS>containing the types added here.
Would those of you interested (@anajavi, @veloek) mind giving them a whirl?

Works nicely at my end.


Support for customized functions with return of string and another types

Hello excellent parser but lack of functions of String, LocalDateTime, maybe better support of override o create customized functions with return of specific types not only LazyNumber.

There is a limited string support, see the readme. But this library is mainly meant for numerical epressions.

@sirlordt Converting the date/time to a Unix timestamp would make it possible to be processed. I mean, that would involve quite some "preprocessing" if you allow to enter the equation, but should be rather straight forward.

I'll close this until somebody comes up with a good solution for general string support


Incorrect slash command name restrictions

In https://github.com/Discord-Net-Labs/Discord.Net-Labs/blob/release/3.x/src/Discord.Net.Core/Entities/Interactions/SlashCommandBuilder.cs there are multiple Regex.IsMatch(value, @"^[\w-]{3,32}$") is the minimum length of 3 correct? The Discord Docs indicate the minimum should be 1. ref https://discord.com/developers/docs/interactions/application-commands#application-command-object

I believe it was originally 3 when this was implemented and later dropped to 1


Fix packaging for WHL format

Context
Join me, on my descent into madness.
We have a problem: we want users to be able to copy and run our built-in recipes with our built-in configs without requiring users to clone the repository. That means that these recipe files and config files need to be available in the wheel package (the preferred and default distribution format for Python packages). Alright, so how do we make sure these files are included in the end wheel package? This is where things get more complicated.
We've made the conscious decision to formulate the relationship between recipes and torchtune as follows: recipes build upon the reusable components in torchtune. Therefore, recipes should not be an importable Python package. So how do we ensure that recipes are not an importable package? We remove them from discovery via our packaging tools. Now, we have solved one problem and created two others: we can no longer import anything from recipes for testing AND the recipe and config files are no longer included in our package, which means we cannot copy or run them.
New Problem A (Testing): This was actually solved via some awesome work by @ebsmothers in #537. The solution was essentially to utilize a subprocess to call the recipes via the command line, similar to how a user would do it. Therefore, we had no reason to import tests anymore.
New Problem B (Missing files in distribution): The core of this question is actually how do we include non-package data in a package? Surely there must be some command or specification that you can turn on to include information. Introducing, the MANIFEST.in file! We can simply add any files we want to include in the distribution with a command like include recipe_a.py and it will be added. For awhile, this seemed to solve all our problems and all was good and beautiful in the magical kingdom of TorchTune. But this was all a lie, for while this method worked while creating a TAR-format distribution, it failed when tested with a WHL-format. Two seconds of digging revealed an immediate problem: MANIFEST.in does not affect binary distributions such as wheels.. Okay, okay, so we just needed to find another way to package non-package data, right? Turns out, PyPA really really reaaaaaaally does not want us to do this and therefore deprecated this ability. I spent some time trying to hack around this limitation, to no concrete avail.

I was actually able to find a way to use deprecated commands to include non-package data in the final WHL, but there was no way to easy find these files using the distributed package. Also, relying on deprecated commands is ... not ideal.

New Problem B-revised (Missing files in WHL): When your back is against the wall, I always recommend giving up. In this case, that meant stopping the attempt to fight PyPA and do what they want: recipes must be included as a package. But author - won't users then be able to import from recipes and run amuck of our beautiful code? Yes, dear user, that is the implication. But, when all seems lost, there emerges a hero: @NicolasHug. A little while ago, he worked on   #594. Although this PR's primary purpose was to prevent tests from being packaged, he incorporated a "hack", which allowed recipes to be "packaged" by a local install, but raised a deliberate ModuleNotFound error if any errant user attempted to import something from recipes. The principle remains the same for packaging the recipes in a wheel.
To summarize: This PR adds the recipes folder to the list of folders packaged into Torchtune.
FAQs
What is the PyPA?
Python packaging standards are maintained by PyPA - the Python Packaging Authority. Packaging in Python has gone through a number of changes in the last years. First, users could use distutils to build and package their Python application. Then distutils was deprecated, and setuptools became dominant. Now, there are several open-source build-backends like poetry and hatch. The point I'm trying to convey here is that Python packaging is constantly evolving. My goal is attempt to follow the PyPA's advice on packaging to set us up for success in the future. I aim to make the most sustainable and robust changes for the repo, rather than try to fit to a fad or newest standard.
Why are there other changes in this PR? Those does seem related to the problem above
While going deep on all the proper Python packaging standards, I discovered that the recommended way to configure the packaging of an application is actually through the TOML file, rather than the setup.py file. Therefore, I made the necessary changes, which also allowed us to delete some extra top-level files.
Changelog

Move all information in setup.py to pyproject.toml
Added recipes to included directory in package
Added version to torchtune init

Test plan

[x] Confirm WHL looks correct

pip install build # install build tool
python -m build # create WHL
Create a fresh conda env
pip install torchtune/dist/torchtune-0.0.1-py3-none-any.whl # install the built package
tune cp full_finetune_single_device . # confirm the files are there and we can locate them
python -c "import recipes" # confirm this throws an error
python -c "import tests" # confirm this throws an error

[x] CI Green
[x] Can be packaged and uploaded via test-infra workflows (see #663)

Thanks Joe for all this work.
So with this PR, the recipes package gets installed when doing pip install torchtune, but import recipes throws an error with a decent error message.
At this point, and to avoid polluting the users' importable space with an unusable recipes package, should we just put recipes within torchtune/recipes? We can still raise the same error message, and we can still have the recipes folder be a top-level folder by just having a symlink? This way recipes isn't a package anymore, only torchtune.recipes exists.

Alternatively, I think we'd be fine by just providing sdists instead of wheels. Looking at the Advantages of wheels that you shared offline, I don't think that any of these points are of importance for torchtune (at least at this stage). It wouldn't be too hard for us to not rely on the test-infra setup and just build/ship our own sdists.

I discovered that the recommended way to configure the packaging of an application is actually through the TOML file, rather than the setup.py file. Therefore, I made the necessary changes, which also allowed us to delete some extra top-level files.

Sort of... But the majority of other pytorch projects still rely on setupy.py and you might get better support from our teams sticking to it (internal link for more context). Maybe @atalman can chime in on that aspect?
(Regardless, if it's not too much work/pain, it might be best to do the transition to pyproject.toml on a separate PR to really understand what's going on with atomic changes)

At this point, and to avoid polluting the users' importable space with an unusable recipes package, should we just put recipes within torchtune/recipes? We can still raise the same error message, and we can still have the recipes folder be a top-level folder by just having a symlink? This way recipes isn't a package anymore, only torchtune.recipes exists.

Forgive my ignorance, but would a symlink work the same way from both a git clone installation and a pip installation? I want to make sure the user experience and how someone references the recipes is the same in both cases.

Alternatively, I think we'd be fine by just providing sdists instead of wheels. Looking at the Advantages of wheels that you shared offline, I don't think that any of these points are of importance for torchtune (at least at this stage). It wouldn't be too hard for us to not rely on the test-infra setup and just build/ship our own sdists.

This is a good point, and although we would eventually want a WHL, I can see that it's not necessary right now. The sticking point from my perspective is that I have a lot more confidence in building and uploading entirely via test-infra's setup, which has a plethora of testing, rather than my own custom workflow. (You can see my attempt at this in the history of commits on #663)

Sort of... But the majority of other pytorch projects still rely on setupy.py and you might get better support from our teams sticking to it (internal link for more context). Maybe @atalman can chime in on that aspect? (Regardless, if it's not too much work/pain, it might be best to do the transition to pyproject.toml on a separate PR to really understand what's going on with atomic changes)

Thanks for sharing the link! Regardless of what we go with, you're absolutely right, I should move this change to a separate PR.

Forgive my ignorance, but would a symlink work the same way from both a git clone installation and a pip installation? I want to make sure the user experience and how someone references the recipes is the same in both cases.

Not sure I understand the concern completely but from my current understanding, this is a non-issue. When/if we move the recipes in torchtune/recipes (or even torchtune/_recipes), all the CLI tools and stuff would be adapted to rely on that. Then we can add a recipes symlink from the project root that points to torhtune/[_]recipes/, but the goal of that symlink is only for the recipes to be visible for users just browsing the repo on the web. It doesn't/shouldn't change anything with how recipes are used by torchtune in general. Or am I missing something?

But the majority of other pytorch projects still rely on setupy.py and you might get better support from our teams sticking to it (internal link for more context). Maybe @atalman can chime in on that aspect?

BTW Andrey confirmed offline that relying on pyproject.roml this is OK with releng, so no issue here.

Not sure I understand the concern completely but from my current understanding, this is a non-issue. When/if we move the recipes in torchtune/recipes (or even torchtune/_recipes), all the CLI tools and stuff would be adapted to rely on that. Then we can add a recipes symlink from the project root that points to torhtune/[_]recipes/, but the goal of that symlink is only for the recipes to be visible for users just browsing the repo on the web. It doesn't/shouldn't change anything with how recipes are used by torchtune in general. Or am I missing something?

Nope, that makes sense! I filed #667 to track this.


feat(client-presence): initial telemetry

Add monitoring context Presence
Two events:

PresenceInitiated logged when runtime instantiates Presence feature.
JoinResponse logged when a client manages response to another client's Join message.

Neither of those can (well shouldn't) hit frequently. The first event should be no more than once per run of client. The second is in response to other clients joining session.
There is a massive test debt with the experimental package. We have API compilation tests, but not execution tests. We'll definitely want to be checking telemetry when we can get there.


Add Netlify deployment guide

https://www.prisma.io/docs/guides/deployment/deployment-guides/deploying-to-netlify?

@janpio updated the name of the issue. Should have been "Update existing Netlify deployment guide"


WinGet Prospects?

WinGet is quite mainstream now and a lot of packages are available in the official Microsoft repositories.
There's some notable benefits to WinGet such as:

Most packages are maintained by their own upstream (this avoids the common issue of the Chocolatey package being out of date)
A real packaging format (MSIX, in the same way Debian has deb and Fedora has rpm files)

MSIX is supported all the way back to Windows 7 through MSIX Core

Comes installed on Windows by default (through App Installer) and officially supported by Microsoft

The only con I can think of is that I don't think WinGet supports Windows 8/8.1. However, that version of Windows has been out of support now for a while anyway. There may also be some packages used by Flare VM which haven't made their way to official WinGet repositories yet (this should probably be checked as the first step).
I'm just throwing the idea out there to get Flare VM's prospects on this matter. If a port is planned then it would probably take a decent amount of effort to get done.
Thanks!

Hey, thanks for the great idea. That could be a way forward and I think we'd be open to support this - especially with contributions from the community around it. First, being the research on package availability on WinGet vs. Chocolatey.

Winget can be added for some packages or updates..
On my system i use both as a more complete upgrade packages..
Also powershell scripts or libraries to automagic-icate this too..  on github ..
At times winget is = to choco or >= a tad newer.
However winget has certificate 📌 📍 pinned issue..
Which one has to force kill the defacto repo... and force   use a certbot enabled.. proxy  (work arround) ms-cert issued/pinned  cert has expired... , thier are a few  tray-gui tools one based in python.. or etc..
Or etc , that will run chocolatey,  winget node.js , paket /cake. .net (dotnet building deps) , *etc updates onto system.. at start-up.. /login

The package rewrite can be in a way quite straight forward as there share the similar pattern (XML on Chocolatey vs YAML on winget). At the same time, we can also set our own YAML file so that winget reads the config & perform the installation accordingly (Source: https://learn.microsoft.com/en-us/windows/package-manager/winget/install#options). In my personal experience also has a higher successful rate in terms of installation as compared to Chocolatey (I always need to perform manual installation for certain tools because they have failed due to hash mismatch, or the link not working as taken down by original author etc). Winget can defintely helps in making installation more sustainable & stable (especially for Microsoft related applications).


Google Batch restarts jobs

In recent tasks, Batch will start to pull the initial Docker Image again and then silently fail.

It happens in any region and since the VMs are destroyed, I'm not sure where to look for clues of what is going on.
https://github.com/filecoin-project/filet/issues/24#tasklist-block-3a7cfb80-b6f8-4558-b458-049e9398661c

These errors were caused by Google Cloud preemting the instance. Using STANDARD as provisioning model fixes it but increases costs potentially by 8x.