Catch GET request to bare sso location early

This issue is imported from pivotal - Originaly created at Feb 20, 2019 by Thijs Kinkhorst
Access the SSO location of Engineblock with a GET request without any parameters
(e.g.:
https://engine.test2.surfconext.nl/authentication/idp/single-sign-on/key:test-20181127
)
Logged is:
engineblock.ERROR: Unable to find the current binding.
Highly cryptic and seems quite far in the processing chain. Check early if there's at least a SAMLRequest parameter present and fail early and with concrete exception if not. (Note that this endpoint supports both GET and POST).

HTTP-code met indicatie dat je iets fout doet ipv dat het een server error. (Thijs Kinkhorst - Feb 26, 2019)

Some findings:

EngineBlock_Corto_Module_Bindings is used to receive a SAML request (authn or response).  She uses the saml2 Binding::getCurrentBinding method to determine whether or not the request is valid. The saml2 Binding::getCurrentBinding throws the Unable to find the current binding. exception.
Our \EngineBlock_Corto_Module_Bindings::receiveRequest method throws a EngineBlock_Corto_Module_Bindings_UnsupportedBindingException, resulting in an exception message specific for ACS. This can be seen as a bug in EB right now, as \EngineBlock_Corto_Module_Bindings::receiveRequest is also used in SSO context, and you don't want a ACS error page when processing an AuthnRequest.

How to make this more robust?

Option 1: By rewriting. And to be specific, by removing the EngineBlock_Corto_Module_Bindings and implement a modern equivelent in Symfony.
Option 2: By adding a validation step to the \OpenConext\EngineBlockBundle\Controller\IdentityProviderController::singleSignOnAction where a helper class determines if the incomming AuthnRequest is supported by EB. After which it is handed over to Corto to receive the request. What will we validate?

Is the correct binding used (Redirect), if not show a user friendly binding not supported exception.
Are the required parameter(s) present (SAMLRequest). If not, show a user friendly error message.
Do we want other custom error messages? For example when the SSO endpoint is used for ACS?

Option 3: By improving the behaviour of: EngineBlock_Corto_Module_Bindings

My current preference is Option 2, as Option 1 will require some more carefull planning and might quickly result in the refactoring of more than just the Bindings module.. Option 3 seems the easiest way out, but does not stimulate moving more logic to the Symfony side of EB. (Michiel Kodde - Mar 6, 2019)

@thijskh, @bartgeesink, @pmeulen

Step one of the change is finished. This entailed improving the error messages. For example. we now log the following exception when a 'bare' GET request is performed on the SSO endpoint:
Mar 13 08:47:52 apps EBLOG[5845]: [2019-03-13 08:47:52] app.INFO: Handling incoming request: GET /authentication/idp/single-sign-on {"session_id":null,"request_id":"5c88b5a8763d8"} []
.............
Mar 13 08:47:52 apps EBLOG[5845]: [2019-03-13 08:47:52] engineblock.ERROR: Unsupported binding, no SAMLRequest was found in the request parameters | Caught Unhandled generic exception

On to the next step: improving the error page. Have you thought about the texts we want to display in the following error situations?

Invalid request verb is used anyting other than a GET request will result in an InvalidRequestMethodException with message: The request method "[used verb]" is not supported on the SSO endpoint
Invalid binding attempted results in: InvalidBindingException. With message: Unsupported binding, no SAMLRequest was found in the request parameters. This occurs when a GET request is fired, but no valid binding could be determined by SAML2.
Unsupported binding was attempted: Results in InvalidBindingException, with message: The binding type "SAML2\HTTPArtifact" is not supported on the SSO endpoint.

Please provide me with appropriate user facing error messages that can be displayed on the error feedback page.
I suggest re-using the existing /authentication/feedback/unable-to-receive-message feedback page for this message. Or do you prefer the introduction of a new route? (Michiel Kodde - Mar 13, 2019)

I propose the following:
"No authentication request received."

The request method "[used verb]" is not supported on the SSO endpoint.
No SAMLRequest parameter was found in the request parameters.
The binding type "SAML2\HTTPArtifact" is not supported on the SSO endpoint.

Please try again."
Keep it relatively technical and concrete because it is helpful for a developer to know what they're doing wrong..
However, do note that although we do not publish that in metadata, we do actually support POST to sso endpoint currently. So 1 and 3 would not be triggering on that scenario. But indeed on "HEAD" or artifact-binding. (Thijs Kinkhorst - Mar 13, 2019)

I like @thijskh his ideas above. Some additional suggestions:
Insert the protocol names in the messages. i.e.:

The HTTP request method "[used verb]" is not supported on this SAML SSO endpoint.
No SAMLRequest parameter was found in the HTTP request parameters.
(#2 is only for GET (HTTP-Redirect), what about the HTTP-POST binding?)
The urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact binding is not supported on this SAML SSO endpoint.
(People do this? Anyway, the use of the binding is easily diagnosed by the presence of the "SAMLart" HTTP GET parameter)

Finally state what is allowed / expected:
Expected a SAML 2.0 AuthnRequest using the urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect or urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST binding on this SAML SSO endpoint. (Pieter van der Meulen - Mar 13, 2019)

@pmeulen Both GET and POST binding need to have a SAMLRequest parameter, right? So 2 can be for both. (Thijs Kinkhorst - Mar 13, 2019)

Yes, that was a regression I was about to introduce. I was under the impression, SSO only supported HTTPRedirect binding. This is now corrected.
I now suggest for message 2:

'No SAMLRequest parameter was found in the HTTP "[used verb]" request parameters' (Michiel Kodde - Mar 13, 2019)

@pmeulen Your suggestion to display the correct binding name `urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact` is not super trivial. The `SAML2\HTTPArtifact` part was merely an example and could also be SOAP. A reverse lookup from SAML2 binding to Urn would be required. This ofcourse is no big deal, but is the additional coupling to the SAML2 library worth it? (Michiel Kodde - Mar 13, 2019)

@michielkodde IMO not, because I don\'t think this is common mistake to make. However, if you use your implementation #2 (do the checks before entering the saml lib, it shouldn\'t be that hard right?). Even then in nice to have territory IMO. (Pieter van der Meulen - Mar 13, 2019)

@thijskh With a HTTP POST I usually call them fields (as it\'s urlencoded from data). I\'m not sure what the correct term is actually. As long as people undestand it, it\'s fine. (Pieter van der Meulen - Mar 13, 2019)

(Michiel Kodde - Mar 13, 2019)

@thijskh Can we get away with displaying the error message in English between the different languages? If not, I will make the exception text translatable. (Michiel Kodde - Mar 13, 2019)

We can get away with it. (Thijs Kinkhorst - Mar 13, 2019)

It seems to have broken unsolicited-sso-requests.
E.g. https://engine.test2.surfconext.nl/authentication/idp/unsolicited-single-sign-on/ba573f07093978e3852ddef0d2465b84/key:20181127?sp-entity-id=https://profile.test2.surfconext.nl/authentication/metadata
now triggers this error. (Thijs Kinkhorst - May 3, 2019)

The request validation added for the solicited SSO was also applied on the unsolicited SSO endpoint. This caused the problem, as the unsolicited SSO request is different in many aspects. This feature was removed for now in this PR
I created #165821300 to add proper request validation for the unsolicited SSO endpoint.
FYI unsolicited SSO is not covered in acceptance tests. (Michiel Kodde - May 6, 2019)

The temporary solution is merged to release/5.10 and master. (Michiel Kodde - May 7, 2019)


Add number of rows to EntitySet.plot()

The repr of an EntitySet shows the number of rows.
import featuretools as ft
es = ft.demo.load_mock_customer(return_entityset=True)
es

Entityset: transactions
  Entities:
    transactions [Rows: 500, Columns: 5]
    products [Rows: 5, Columns: 2]
    sessions [Rows: 35, Columns: 4]
    customers [Rows: 5, Columns: 4]
  Relationships:
    transactions.product_id -> products.product_id
    transactions.session_id -> sessions.session_id
    sessions.customer_id -> customers.customer_id

The .plot() method is frequently preferred by users since it shows the same information and more visually, with the exception of the number of rows.

To improve .plot(), we should decide on a way to add the number of rows to the output

I'd be interested in trying this...one idea could be to just add it in brackets. So where it says transactions, it could instead say transactions (500 rows), and similarly for the others


Refactor errors

determine the pattern for all daemon errors
differentiate between internal errors and user errors
update all errors to use this pattern
all user errors should have string constants in addition to messages

Work in progress here: https://github.com/lbryio/lbry/wiki/Error-Codes


Use DrawElement with a IntBuffer (transformed in ByteBuffer) for Indexes of element

I think we can increase the performance during the render loop.
Actually when we create a cube, for exemple, we need generate and display 36 vertices, 36 texture coordinates, 36 colors and 36 normals, but in fact for make a cube we just need to generate and display 8 of each.
VBO can display objects with a list of unique vertices, a list of unique texture coordinates, a list of unique color, a list of unique normals and a list of unique indexes.
For exemple, for a cube we have :

8 vertices, 8 texture coordinates, 8 colors, and 8 normals
36 indexes of render

It's better for memory, and better for the render loop.
What do you think about it ?

This is possible, but might be problematic. Assume the case of coloured cube, with one color per face, you can't have indices since there will not be unique colours per vertex. The same happens with textured models too.

Maybe use a state for the Batcher who will be true when the batcher is in DrawElement mode with indexes.
And add new function index(int p1Index) like the vertex(float p1X, float p2Y, float p3Z). I've tried it the last night (with fast and not really brainned code), but it was not working...
I will try with more reflexion this week.

Closing this for now, as there are more challenges than the advantages this provides.


Update kinesis to 2.21.0

About this PR
📦 Updates software.amazon.awssdk:kinesis from 2.20.162 to 2.21.0
Usage
✅ Please merge!
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.
Configure Scala Steward for your repository with a .scala-steward.conf file.
Have a fantastic day writing Scala!

⚙ Adjust future updates
Add this to your .scala-steward.conf file to ignore future updates of this dependency:
updates.ignore = [ { groupId = "software.amazon.awssdk", artifactId = "kinesis" } ]

Or, add this to slow down future updates of this dependency:
dependencyOverrides = [{
  pullRequests = { frequency = "30 days" },
  dependency = { groupId = "software.amazon.awssdk", artifactId = "kinesis" }
}]

labels: test-library-update, early-semver-minor, semver-spec-minor, commit-count:1

Superseded by #617.


new 500 error page

500 error page implemented in front-end instead of rendering the error code alone. It took some time since it was tricky to find where and how the errors were handled. It looks like the front-end code is actually a fork from the back-end and for this reason these errors were handled in an error_controller.
Trello card:
https://trello.com/c/u7SjOx3a/519-tariff17-as-a-site-visitor-i-want-to-be-displayed-a-500-error-instead-of-being-redirecting-to-index

Well done :+1:


[Bug]: On Composer install of Verbs itself, "bugg" repo is private

What happened?
Upon trying to install Verbs itself, to check out the tests and such, I get this:

Could not fetch https://api.github.com/repos/DanielCoulbourne/bugg, please review your configured GitHub OAuth token or enter a new one to access private repos

How to reproduce the bug

Check out this repo.
composer install

Package Version
6f50d61
PHP Version
8.1.26
Laravel Version
x.x.x
Which operating systems does with happen with?
macOS, Windows, Linux
Notes
No response

I've removed it!


[$1000] Web - User avatar is still shown after thread reply has been removed after being flagged

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Action Performed:

Send a message from account A to account B
Reply in thread from account B
Go over to account A and click on the reply button > click the flag icon
Flag the comment sent from account B as harassment and wait for the comment to be removed
Go back to the original chat and observe the reply button

Expected Result:
The reply button and the avatar should be removed since Account B's reply has been removed after flagging
Actual Result:
The 1 Reply message and the avatar is still being shown after being removed because of being flagged
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[X] Android / native
[X] Android / Chrome
[X] iOS / native
[X] iOS / Safari
[x] MacOS / Chrome / Safari
[X] MacOS / Desktop

Version Number: 1.3.35-5
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://github.com/Expensify/App/assets/93399543/f8a60f61-1b19-4676-b8db-aefb91c898da
https://github.com/Expensify/App/assets/93399543/d1547512-bc9a-471c-bb9d-32d90c125b10

Expensify/Expensify Issue URL:
Issue reported by: @Nathan-Mulugeta
Slack : https://expensify.slack.com/archives/C049HHMV9SM/p1688210429646659
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01300081e9977f4be7
Upwork Job ID:<PHONE_NUMBER>203498496
Last Price Increase: 2023-07-14

I can reproduce but I think it's related to https://github.com/Expensify/App/issues/21497
Holding until regression in https://github.com/Expensify/App/issues/21497 is fixed to see if this is resolved.

FIle ReportActionItem.js. This data that is fetched from backend will have to updated by the backend to exlude message that are flagged. Or if frontend only solution is required. I can fetch the child report and then figure out what data to show.

Reviewing and receiving proposals! cc @Santhosh-Sellavel

@ygshbht Can you write a formal proposal please

Proposal
Please re-state the problem that we are trying to solve in this issue.
The user's avatar is still displayed in thread replies even after messages have been deleted.
What is the root cause of that problem?
Endpoint /api?command=OpenReport returns the accountID of the deleted message User on childOldestFourAccountIDs even after the message is deleted.
What changes do you think we should make in order to solve the problem?
If we can´t change backend code to filter out the accountIds that are not in the thread anymore, we can add a new method to get the accountIds for a specific report thread on https://github.com/Expensify/App/blob/main/src/libs/ReportActionsUtils.js
/**
 *
 * @param {String} reportID
 * @returns {Number[]}
 */
function getThreadAccountIds(reportID) {
    const threadActions = lodashGet(allReportActions, [reportID]);
    const chatActions = _.filter(
        threadActions,
        (action) => action.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT && !isPendingRemove(action) && action.actorAccountID !== CONST.ACCOUNT_ID.CONCIERGE,
    );

    return _.uniq(_.map(chatActions, (action) => action.actorAccountID));
}

And use it on ReportActionItem.js
https://github.com/Expensify/App/blob/22b9aa44406880bae1432552be3466337f3490ba/src/pages/home/report/ReportActionItem.js#L340
Change to:
const oldestFourAccountIDs = ReportActionsUtils.getThreadActors(props.action.childReportID).slice(0, 4);
What alternative solutions did you explore? (Optional)
Do the change on backend code to fix childOldestFourAccountIDs field on report, filtering out accountIds that are not in the thread anymore.

Contributor details
Your Expensify account email<EMAIL_ADDRESS>Upwork Profile Link: https://www.upwork.com/freelancers/~0172e10cd6c14a9a95

I think the change in the backend is the way to go here based on this proposal.
Marking reviewed to get a CME
C+ Reviewed
🎀 👀 🎀

@srikarparsi
I think the change in the backend is the way to go here, Let us know your thoughts on this proposal too. Thanks!

I'm going to be ooo until August 31st so going to unassign and assign a new teammate.
At this time, we're receiving and reviewing proposals and maybe accepting this https://github.com/Expensify/App/issues/22072#issuecomment-1636071202

Thanks @Santhosh-Sellavel and @0rdep! The backend change seems like the right approach to me as well but I will look more into the code tomorrow and update the issue.

Working on familiarizing myself with the backend code with threads. Should have a draft PR up by end of week.

Not overdue waiting for @srikarparsi

Hey! Worked on this a little today and will test/push tomorrow.

Thanks @srikarparsi - what is the latest on this for this week please?

Tested and investigated more and found out that it's not a problem with the user avatar but rather the hasReplies line. I'm looking into a fix in Auth since the childVisibleActionCount should be 0 if there's only one comment and it's been reported.
https://github.com/Expensify/App/assets/48188732/f4f2074c-aa5d-46ba-b9fb-756629a33552

Auth PR deployed, web pr reviewed and needs to get merged.

Tested on production and it looks good. This is ready to be closed out @Christinadobrzyn if there is no payment needed
https://github.com/Expensify/App/assets/48188732/ea6b50d4-702f-40c4-9649-2792d483d622

Reporting bonus has not been processed yet.

Hey @Christinadobrzyn can payment be processed for this issue? Thanks

@Santhosh-Sellavel can you confirm you're paid through New Expensify? If that's true, I'll close out the Upwork job posting and this GH.

Thanks for confirming @Santhosh-Sellavel - updated the payment here
Closing the job in Upwork and this GH. Feel free to let me know if I'm missing anything!


updateWorkflowOutput optimize

updateWorkflowOutput, if output parameters null, the output and status of the last task in the workflow will be copied to workflow output.

I'm closing this PR. But, please feel free to reopen the conversation in gitter channel if you think we've missed anything.


Allow bulletActiveClass etc to be multiple classnames

Is your feature request related to a problem? Please describe.
For people who use utility CSS like TailwindCSS, it is frustrating to have to work with only one classnames.
Describe the solution you'd like
instead of bulletActiveClass: "one-class"
allow for : bulletActiveClass: "bg-gray-800 w-8 h-1 inline-flex rounded-full transition duration-300 ease-in-out"
Describe alternatives you've considered
An alternative solution for me atm is to go back to regular css, one class 😐
Additional context
None

It is supported

its not working on version 11.0.5


[WIP] Adds some initial work for submitting consignments

I need to make some metaphysics changes before I can wrap this up, so submitting this for momentum.

Fails

:no_entry_sign:

No CHANGELOG added.

  Generated by :no_entry_sign: dangerJS


Update README.md

@IlyaFaer would you be ok to re-create this branch/PR in a fork? We shouldn't really be developing in the main repo anymore. I may be required to set some setting on this repo soon that will stop us from being able to do this.

@skuruppu, oh-oh. Yeah, I can do this, here is the new PR:
https://github.com/googleapis/python-spanner-sqlalchemy/pull/153
Closing this one.


Create endpoint to allow specification of "VFS-home", "Artifacts-DB" and "Theme" and persist to config.

In support of

These are client-side settings that don't need any server-side support just yet. If anything, SlamData Advanced should have a place to store the settings for identified / authenticated users.
The artifact Isolation will require "soft links" for Quasar, and a ticket already exists for that.

Unclear:

First of all these settings need to be persisted somewhere. Today the only option is config-file or client-side user-storage. Config-file: we are moving to Metastore in preparation for quasar-clustering.

Some of these settings appear to be global (i.e. should not vary based on an individual user). e.g. Isolate Slamdata-artifacts-db is likely to be a system-wide setting independent of users/authors. So that will need a backend REST service to persist. Of course more work needs to happen to actually use it, by the rest of quasar.

Default Theme & Home-Directory-Location within VFS could be user-level setting or a global setting by the Admin across all users. (User level over-rides will be spec'ed later. Themes already have a deck-level over-ride). That is one interpretation, which will again need a REST service to persist.

They can be persisted client-side. Or, if we add a user profile to authenticated users — which would be added in SlamData Advanced, NOT in Quasar — then it could be persisted there. This is NOT related at all to "config file".
The artifact isolation is NOT global. In a cloud setting, such as SlamData Cloud, or in a multi-tenant situation, artifacts may need isolation different users may have different artifact isolation preferences (each customer of SlamData Cloud will have a different artifact isolation directory).
The home directory and theme setting are PER USER settings. While an admin may have control over home directory at some point — within reason, since even the admin may have limited access in a Cloud scenario — that functionality is not part of the MVP and is not spec'd out.

You have copied over the screenshot but none of the notes. The issue will be more clear with the notes attached to the original issue.

Thanks @jdegoes. Yes should have added detailed notes.

The artifact isolation is NOT global. In a cloud setting, such as SlamData Cloud, or in a multi-tenant situation.

You raise a good point here, which is that the scope of settings is a function of deployment modality. i.e.

DESKTOP -> The single author is the Admin - so all Admin functionality is likely available to the desktop user.
CLOUD/MULTI-TENANT -> We do not have any deployments of this nature today, but yes, we would have to accommodate either @ "user" scope or Create an altogether new concept such as "tenant" or "partition" where each "tenant/partition" could have multiple "users".  There may be benefits to not overloading "tenants" as "users".
ENTERPRISE -> which is ALL of our deployments today, I would expect artifact-isolation to be global/system-wide - and not give each author a preference to dictate where Slamdata artifacts go (since it will just increase the number of potential failure points and complicate the persistence and operational-management story within the enterprise).

would be added in SlamData Advanced, NOT in Quasar
Agreed. Would be good to specify this as well. e.g. The interpretation we are working with is that artifact isolation is available within OSS Quasar as well, which couple with the previous point, would mean that it does go into the "config-file" of today...

It seems that to minimize confusion and multiple interpretations, we would need for each of the settings - additional metadata that details
Scope

Session-level, User-level (persists across multiple sessions), Tenant-level or Global.
Orthogonally scope could relate to the entire cluster of Quasars or be specific to 1 Quasar in the Quasar-cluster (this presumably could be handled via just command level JVM over-rides).

Persistence (client-side, server-side (within config)):

Since any setting is application "configuration".
If it is being stored server-side, it is in the "config file" (that is the only place to persist unless it is transient and stored just in memory and not expected to survive a server-restart).
Most "config file" will move to Metastore to support Quasar-clustering
Except those settings that will relate to a specific node in the Quasar-cluster (e.g. different JVM settings, or running 1 as a canary (where it has different log-levels etc.)).

I am taking a shot at this and will add it to the epic - so you can clarify further: Something like a table -> Parameter | Scope | Persistence @ Deployment Modality.

@zeeiyerWork Again, these are client-side settings for now. You can work on specs for an API (NOT in Quasar, but SlamData Advanced), which lets the front-end reliably store arbitrary "profile" data for authenticated users. That would be quite useful and is a small amount of work. Beyond that, no additional specs are required.

Engineering is not in agreement with Product on slamdata-artifacts-db being a client-side setting (i.e. a setting with scope = specific-user-browser on the client-side). So we will skip implementation of that for 4.3 pending more rigorous specifications/discussions.
The reasons are the following:

The "text" promises something, but the implementation as described by you, will fail to deliver it. Since if stored as user-storage, and I as an author open up a new browser, i no longer have the Slamdata-artifacts-db setting and I will now "pollute" the production db with slamdata artifacts.

Again, if even if one uses the same browser, some authors may set it and some may not - and thus the promise of no "pollution" of the production db remains unmet.
From an implementation perspective, anyways - the frontend needs to send this along as the targetDB, for it to take effect, which wasn't anyways in scope.

Even with home-directory-setting and default-theme, one could make a case that these are global level settings (i.e. defaults) which an "ADMIN" sets. (i.e. have the notion of an Admin group - user belonging to that group will see all tabs and set defaults). A given end-user may override some (and sees only My-Settings). But for now, one can just buy into a less sophisticated implementation and just say that these (home-directory and default-theme) are just user-level settings (the admin does not get to set any defaults in this regard).

This issue was moved to slamdata/slamdata-backend#214


match_dense confs on EfficientLoftr

I got bad results when I try to test EfficientLoftr on aachen_v1.1 datasets by using the config:
    "loftr_aachen": {
        "output": "matches-loftr_aachen",
        "model": {"name": "loftr", "weights": "outdoor"},
        "preprocessing": {"grayscale": True, "resize_max": 1024, "dfactor": 8},
        "max_error": 2,  # max error for assigned keypoints (in px)
        "cell_size": 8,  # size of quantization patch (max 1 kp/patch)
    },

I wonder to know how to chagnge "max_error" and "cell_size". Do you have any plan to update the EfficinetLoftr test?

Hello, I also want to test EfficientLoFTR, but I encountered an error where two tensor sizes do not match at the transformer position. How did you solve this? Or could you share your testing script with me? Thank you.

May be you should try to change "dfactor": 8 to "dfactor": 32

Hello, I also want to test EfficientLoFTR, but I encountered an error where two tensor sizes do not match at the transformer position. How did you solve this? Or could you share your testing script with me? Thank you.

Thank you for your reply, I'm not quite clear what parameter "dfactor" refers to.

Thank you for your reply. Could you please explain what the "dfactor" parameter represents above?


Enhance markdown in monailabel notebooks

Description
Add notes in monailabel related notebooks to tell users terminate the kernel before closing the notebook to avoid port already used issue.
Checks

[ ] Avoid including large-size files in the PR.
[ ] Clean up long text outputs from code cells in the notebook.
[ ] For security purposes, please check the contents and remove any sensitive info such as user names and private key.
[ ] Ensure (1) hyperlinks and markdown anchors are working (2) use relative paths for tutorial repo files (3) put figure and graphs in the ./figure folder
[ ] Notebook runs automatically ./runner.sh -t <path to .ipynb file>

The notes and added statements look good.
The monailabel notebooks can not be included in the notebook tests, before we listed them in the test file.
Thanks for the PR.


Provide additional network info on "adopt" topic

When rebooted, we get a message on the adopt topic with the following:

Do you think it would be useful to
a) also provide wireless signal and channel?
b) implement a periodic state message with the above info?
With tasmotas, I have found this a really good way of monitoring wifi signal, but also very useful for troubleshooting connection issues.

Discussed this with @sumnerboy12 and agreed that periodic messages not really that useful as we can use LWT. Also wifi signal can be monitored elsewhere e.g. from one's wireless controller :) So I'll close this.


Only build gnu_unwind_find_exidx for ARM archecture

Summary
since it's ARM specific and put the code to libs/libc/machine/arm/.
Before change, the build will fail on x86/sim target if c++ exeception is enabled.
Impact
No functionality change
Testing
Please ignore the nxstyle false alarm(__EIT_entry):
libs/libc/machine/arm/gnu_unwind_find_exidx.c:46:15: error: Mixed case identifier found

@xiaoxiang781216 should we white list __EIT_entry?

Done.

Can anyone approve this patch which block libcxx/uClibc++ on sim?

@davids5 I have addressed your concern. Do you have any more issue? If not, please merge this patch.


"New anonymous class definition will be duplicated at each inline site" warning

set scala 3.3.4-RC1 and core/compile

https://github.com/scala/scala3/commit/ffaab87e6caa63dfa837d42679883128774eb00f
https://github.com/scala/scala3/issues/16723

https://github.com/tarao/record4s/blob/3f127ed33099c485e08ccd1df1261c38d641e1fe/modules/core/src/main/scala/com/github/tarao/record4s/Converter.scala#L46-L56
[error] -- [E197] Potential Issue Error: my-record4s-project-dir/modules/core/src/main/scala/com/github/tarao/record4s/Converter.scala:46:4
[error] 46 |    new Converter {
[error]    |    ^
[error]    |   New anonymous class definition will be duplicated at each inline site
[error]    |
[error]    | longer explanation available when compiling with `-explain`
[error] -- [E197] Potential Issue Error: my-record4s-project-dir/modules/core/src/main/scala/com/github/tarao/record4s/Converter.scala:56:23
[error] 56 |  ): Converter[R, T] = new Converter {
[error]    |                       ^
[error]    |   New anonymous class definition will be duplicated at each inline site
[error]    |
[error]    | longer explanation available when compiling with `-explain`

same warnings
https://github.com/tarao/record4s/blob/37f5efc877a8b2b216964bfee66692977b6756c1/modules/core/.js/src/main/scala/com/github/tarao/record4s/RecordPlatformSpecific.scala#L173
https://github.com/tarao/record4s/blob/37f5efc877a8b2b216964bfee66692977b6756c1/modules/circe/src/main/scala/com/github/tarao/record4s/circe/Codec.scala#L70
https://github.com/tarao/record4s/blob/37f5efc877a8b2b216964bfee66692977b6756c1/modules/circe/src/main/scala/com/github/tarao/record4s/circe/Codec.scala#L82


bioconda release?

Are there any plans to create a bioconda recipe for tw?

That sounds like a nice idea! you may want to submit a PR for it!

OK. I didn't want to create a recipe if the tower-cli developers wanted to maintain one themselves.

@adamrtalbot added this in https://github.com/bioconda/bioconda-recipes/pull/41629 🚀


JENKINS-64365: Support SonarQube 7.7 with Pull Request Analysis

Incompatibility with Sonar 7.7
--

Starting with Sonarqube 7.7 the preview mode (-Dsonar.analysis.mode=preview) was removed, making it incompatible with the plugin.

Sonarqube release notes say it now has "native support for short-living branches" https://www.sonarqube.org/sonarqube-7-7/, author will make an effort to integrate these features. Contributions are appreciated

This PR is a contribution to the problem mentioned above. Feedback to this PR is highly appreciated as this is my first contribution to a Jenkins Plugin.
Chosen solution

GUI: Introduce a switch for Pull Request/SQ 7.7 and Preview Mode
Workflow: As JSON file is no longer present, fetch result directly from SonarQube: https://sonarqube-server.com/api/issues/search?componentKeys=myComponent&pullRequest=4711 and create a Report object like formerly based on JSON file.
Add dependency to Token Macro | Jenkins plugin in order to evaluate expressions like ${GERRIT_CHANGE_NUMBER} as pullRequestKey
Add dependency to SonarQube Scanner | Jenkins plugin in order to reuse SonarQube server config.

Hint
As some plugins were not compatible with Jenkins 1.625 I had to upgrade some dependencies. Maybe not all were required, but probably it was a good idea anyway to stay up to date.

Java 7 -> 8
Jenkins parent 2.11 -> 4.4 (this also upgrades the Jenkins version which is used for testing)

Hi @masooh - Not sure if this is getting any attention, have you also logged a Jira? I haven't found one but am interested in getting this merged :) We're currently evaluating Sonar 8.5.1

@svogt  I tried to create an account on Jira (https://issues.jenkins.io/login.jsp), but when I try to login, I'm always getting: "Sorry, your username and password are incorrect - please try again." or "Sorry, your userid is required to answer a CAPTCHA question correctly.". Of course I double checked my password, but with no positive outcome. Due to lack of access to Jira I therefore created this PR directly.

@svogt I tried to create an account on Jira (https://issues.jenkins.io/login.jsp), but when I try to login, I'm always getting: "Sorry, your username and password are incorrect - please try again." or "Sorry, your userid is required to answer a CAPTCHA question correctly.". Of course I double checked my password, but with no positive outcome. Due to lack of access to Jira I therefore created this PR directly.

Oh that's a pity, unfortunately I'm not involved with Jenkins - just stumbled upon this PR as I would like to have that too ;)
AFAIK Jenkins uses a centralized account management system, over at https://accounts.jenkins.io/
Maybe you can sign up there and then use the login in Jira - Hopefully that should work. If that doesn't work either - write here and I'll create an issue for this.

@svogt Thank you for the tipp with https://accounts.jenkins.io/. This worked for me.
Created an issue now: https://issues.jenkins.io/browse/JENKINS-64365

Is this solution based on SonarQube Developer edition and its short live branches ? or does it works with a CE also ?

@JR-Utily it requires the Pull Request Analysis of the developer edition. Not really happy about it either.

@aquarellian - is there something left open here or can this be merged?

We've been using this internally without issues for months now, and we'd be happy if we could switch back to the official plugin release once this is merged.

@masooh  I meant you can't choose it in the UI as the checkbox to activate is in the legacy configuration block. But yes, you are right, this block is untouched, but unreachable, if you configure the pull request analysis via UI.

@masooh I meant you can't choose it in the UI as the checkbox to activate is in the legacy configuration block. But yes, you are right, this block is untouched, but unreachable, if you configure the pull request analysis via UI.

I'm unaware of this configuration, as I'm waiting for this PR to be merged to start using the plugin. :)
But is it a blocking issue with this PR ?
Or something that can be fixed in a later PR ?
In the first case, can you please explain what should be the correct behavior ?

Have anyone tested this PR with https://github.com/mc1arke/sonarqube-community-branch-plugin ?

But is it a blocking issue with this PR ?
Or something that can be fixed in a later PR ?
In the first case, can you please explain what should be the correct behavior ?

IMHO this shouldn't block this PR and can be improved upon later.

But is it a blocking issue with this PR ?
Or something that can be fixed in a later PR ?
In the first case, can you please explain what should be the correct behavior ?

IMHO this shouldn't block this PR and can be improved upon later.

@aquarellian What should be done for this PR to be merged ?

But is it a blocking issue with this PR ?
Or something that can be fixed in a later PR ?
In the first case, can you please explain what should be the correct behavior ?

IMHO this shouldn't block this PR and can be improved upon later.

It's for us just a known limitation and I can't say how many of our builds are affected, but without this PR none of our builds works, so better than nothing ;) I just hoped as the PR is a bit in limbo it could have easily be added before the merge.

Hi All, thanks for the effort for creating this PR. Is there an example or documentation to show us how to use the pull request analysis mode in a declarative pipeline job, where I think UI configuration is not available? Thx

Have anyone tested this PR with https://github.com/mc1arke/sonarqube-community-branch-plugin ?

Yes, it does work with the community branch plugin. I can confirm the PR works with
sonarqube 8.9.2 (community edition)   sonarqube community branch plugin 1.8.0
To answer my own question, the pipeline syntax can be worked out by using the Pipeline Syntax helper.

Please integrate this feature asap, this plugin makes no sense any more without it.

Thank you @masooh for opening the way !
Fixed in #110 :tada:


Cairo environment setup

An introductory Cairo commit. Includes:

a helper shell script for compiling, running against input, and triggering a debugger
basic definitions for AssetOutcome and AllocationEntry
a partial implementation of parsing JSON input data of the above

Note: I accidentally made two commits against main:

cairo.sh
outcomeData.json


Use tbf_protected_region_size to indicate intended load address

This removes the special .tbf_header section previously created by libtock-rs, in favor of setting the tbf_protected_region_size symbol and shifting the application's FLASH section by TBF_HEADER_SIZE.
While adding a .tbf_header section is a convenient way to reserve space for elf2tab to insert a TBF header, it causes issues with recent versions of elf2tab and certain Rust toolchains:

depending on factors such as the Rust toolchain version and the precise FLASH address, the linker may choose to place the RAM section before the FLASH section in the generated ELF file. This is perfectly legal for the linker to do: the intended load address for those sections are still maintained and included in the appropriate section headers. Furthermore, internal references in the ELF file (such as the addresses in the rt_header) will rely on this ordering of sections -- meaning that elf2tab is not allowed to rearrange them. However, when including a .tbf_header section at the start of the FLASH section, this may then not actually be located at the start of the ELF binary itself.

it seems that elf2tab no longer considers the .tbf_header region to not be included in the actual TBF binary (I assume this wasn't always the case, otherwise this would have never worked?). Instead, the .tbf_header section is included and loaded in the final TBF, and as a result the application is offset by an incorrect amount (twice the TBF_HEADER_SIZE)
I'm not sure whether this issue is actually an elf2tab bug in that it seemingly considers NOBITS sections for loading, but those sections are also included in the ELF file, so I assume that elf2tab works correctly here.

This change tries to retain the previous semantics (developers only specify the intended load address in their platform linker scripts), while using a more reliable mechanism for elf2tab to prefix the generated binary with TBF headers and a protected region that, when prepended, aligns to this intended load address.
Most importantly, we exclude the TBF header from any of the app's MEMORY sections, giving the linker free reign over their layout. A special symbol, tbf_protected_region_size, communicates to elf2tab the intended protected region size, which matches the FLASH-section's ORIGIN of $INTENDED_LOAD_ADDRESS + TBF_HEADER.
In essence, this adjusts libtock-rs to behave like libtock-c (where the TBF header is also not reserved in the binary itself), while retaining libtock-rs' more elegant semantics. With this change, as long as elf2tab can reliably detect the app's fixed flash address, RAM address and .start symbol offset, the ELF's particular layout should now be irrelevant for elf2tab.
Relevant elf2tab PR: tock/elf2tab#70

Yes, that's most certainly possible and I agree that it's probably cleaner. I just didn't want the initial changeset to contain any more changes than strictly necessary, and I guess one could bring the concern that not having the MEMORY declaration in the board-specific linker files makes this infrastructure slightly less transparent. Will update accordingly.

Looks like one (or more) of our boards don't pass the new alignment check. I think you should just fix them manually until CI passes.

Rebased. This might be a good test for the GitHub merge queue integration, so let me set that up first and perhaps then we can try to merge this?

SGTM

Looks like one (or more) of our boards don't pass the new alignment check. I think you should just fix them manually until CI passes.

Right, thanks for the heads up. The alignment checks still used the ORIGIN(SRAM) function whereas this PR now changes this to be SRAM_START. Hopefully CI is more happy now. :crossed_fingers:

Fixed the remaining incorrect RAM start offsets. Coincidentally, this issues does not seem to affect ARM binaries (the ELFs look good), but must've been present in quite some RISC-V boards, as they had start addresses not aligned to a 4kB boundary, and I confirmed the generated ELFs to be broken. Maybe this used to work on some older llvm-ld linker version or with the previous elf2tab version (before the ELF parsing rewrite).

Don't think that Bors will work given the elf2tab issues on the current Rust tool chain, but here goes nothing:
bors r+
If this fails, will merge manually.


Does this also work with Web builds?

I saw the working code usesPacketPeerUDP and ENetMultiplayerPeer, and also the PacketHandshake uses over_enet function which uses ENetConnection. If I were to use web builds, and use WebRTC, I am assuming right now it wouldn't work.
Problem I see it would be the web build would use webrtc protocol, while the desktop one uses udp.
Would this be feasible at all to be implemented? Would it require a big change in the server as well?

My line of thinking was that web builds wouldn't need noray or handshake mechanisms, since there is already a server that's delivering the web build. So it would make sense to host the game server there too, and connect to it.
The rest of the code should work fine, as it's all RPCs ( even though I did not test with web builds ).
So to sum it up, it should be fine, you probably won't need to call noray/handshake-related methods in the web build.

But, if for example you would want both web builds and desktop builds, they wouldn't work together, no?

So, the use case would be to have two builds ( desktop and web ), running simultaneously, one using UDP and the other using WebRTC ( or WebSocket ), and all of them playing together?
The challenge here would be to build a server that can accept both types of connections and communicate on them. To me, at first glance, it looks like this is not a netfox challenge but a Godot one. I've seen Fish Networking do this with a multipass transport that takes data on different kinds of connections and passes it to the higher levels, so desktop and web builds could play together transparently.
Not sure how you'd do that in Godot, since you can only use one MultiplayerPeer at once. Probably a custom MultiplayerPeer implementation could work, for example extending MultiplayerPeerExtension from a GDExtension.
Peer to peer multiplayer is currently not supported explicitly by netfox - it may or may not work, can't promise anything regards that.
Sorry for the bad news here :slightly_frowning_face: Building a custom GDExtension might be a solution, but it's not on the roadmap for the foreseeable future. Or using one, if it exists.

But what about running WebRTC on both Desktop and Web builds? There is a module that allows implements WebRTC for Desktop.

if both builds use WebRTC, then you should be good!
netfox itself is protocol-agnostic, as long as you can get a connection going, it should work. By extension this applies to netfox.extras as well.
netfox.noray is specific to noray, and thus to UDP connections.
So if you're not using UDP, you can still use netfox, just not the noray integration for establishing connections.


display(): options > problem displaying more than 3+ fields

If a data set contains more than six fields (see million dollar home sales sample) it's not possible to
choose from all available fields in "fields". Only 6 are displayed and the rest is truncated and cannot be selected. If more than 3 fields are selected as "keys" or "values" only 3 are displayed. Reason: no scroll bars.

@ptitzler scroll bars are present for me:

if you are using a mac you may want to verify your System Preferences... > General > Show scroll bars settings

:+1:


cmd/go:  the "build_plugin_non_main" test case cannot pass for linux/mips64le port

What version of Go are you using (go version)?

$ go version
go1.15.3

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
linux/mips64le
go env Output
$ go env

What did you do?
run go command tests (cmd/go):
bash run.bash --no-rebuild
--- FAIL: TestScript (0.03s)
--- FAIL: TestScript/build_plugin_non_main (0.16s)
script_test.go:210: # Plugins are only supported on linux,cgo and darwin,cgo. (0.152s)
> [!linux] [!darwin] skip
> [!cgo] skip
> go build -n testdep/p2
[stderr]
#
            # testdep/p2
            #
mkdir -p $WORK/b001/
            cat >$WORK/b001/importcfg << 'EOF' # internal
            # import config
            EOF
            cd $WORK/gopath/src/testdep/p2
            /usr/local/go/pkg/tool/linux_mips64le/compile -o $WORK/b001/_pkg_.a -trimpath "$WORK/b001=>" -p testdep/p2 -complete -buildid jfOUz6df0ldy6zplycX5/jfOUz6df0ldy6zplycX5 -goversion go1.15.3 -D "" -importcfg $WORK/b001/importcfg -pack -c=4 ./p2.go
            /usr/local/go/pkg/tool/linux_mips64le/buildid -w $WORK/b001/_pkg_.a # internal
            > ! go build -buildmode=plugin testdep/p2
            [stderr]
            -buildmode=plugin not supported on linux/mips64le
            [exit status 1]
            > stderr '-buildmode=plugin requires exactly one main package'
            FAIL: testdata/script/build_plugin_non_main.txt:7: no match for `(?m)-buildmode=plugin requires exactly one main package` found in stderr
FAIL
FAIL cmd/go 133.150s

What did you expect to see?
the "build_plugin_non_main" test case can pass for linux/mips64le port
What did you see instead?
the "build_plugin_non_main" test case can skip

I found the reason from logs why the test case cannot pass and I will skip the test case on linux/mips64le, is it OK?
-buildmode=plugin not supported on linux/mips64le


remove repeated "if the" from documentation

This is to change a small error in the documentation. There are no changes to code.

@swift-ci please smoke test and merge

Thanks!


add Java V6 code examples

Add code examples for Java library V6 major bump

I'm seeing a lot of discrepancies between passing in a string ID to function OR retrieving the object and running .getId() on it to pass that in. We should pick one and make that consistent throughout. I'm not married to either and will leave it to you.
I'm also seeing a lot of inconcistencies of the naming of params or maps. Some are just called params while some are paramMap and some are the objectMap. Let's make these all the same throughout (prefer params or paramsMap
Hmmm, GitHub isn't showing the diffs like I thought it should for the v5 files, it should have just showed the files were moved but it's showing them all as added. I assume that the v5 files were literally just copy/pasted and didn't have anything change?

1: We need to retrieve the object first in order to use the lowestRate() function, other methods that do not use lowestRate() function should just pass in the ID directly to avoid redundant retrieving object step.
2: The naming came from the previous example, but I can change them to params
3: Yes, v5 files were literally just copied/pasted and didn't have anything change.

1: We need to retrieve the object first in order to use the lowestRate() function, other methods that do not use lowestRate() function should just pass in the ID directly to avoid redundant retrieving object step. 2: The naming came from the previous example, but I can change them to params 3: Yes, v5 files were literally just copied/pasted and didn't have anything change.

Even still, I believe there are more that can/should be converted to one or the other for better consistency. Can you double check to make sure nothing snuck through?
Awesome, thanks
Perfect

1: We need to retrieve the object first in order to use the lowestRate() function, other methods that do not use lowestRate() function should just pass in the ID directly to avoid redundant retrieving object step. 2: The naming came from the previous example, but I can change them to params 3: Yes, v5 files were literally just copied/pasted and didn't have anything change.

Even still, I believe there are more that can/should be converted to one or the other for better consistency. Can you double check to make sure nothing snuck through?
Awesome, thanks
Perfect

How about adding a line of comment to the code examples that retrieve an object which uses lowestRate function later on?

@jchen293 I'm not sure what you mean.

@jchen293 I'm not sure what you mean.

For your first question regarding there are some discrepancies between passing in a string ID to function OR retrieving the object and running .getId() on it to pass that in. I was thinking to add a line of comment to explain why retrieving an object is needed, which is being used later on for buying a shipment/pickup/etc. so you could leverage the lowestRate() feature in a few code examples.

@jchen293 oh gotcha, thanks for clarifying. I don't believe it's necessary. Let's retrieve when needed for things like lowest rate and then use a string elsewhere.


To sort elements in array

Please review my code.

Please review my code.

@aayushi-droid can you please review my code???

Spam Pull request. No issue and Issue no.


Table : unset empty value of tableColumnSearchQueries

I added the unsetting of the key of the $tableColumnSearchQueries because it was still in even when value was empty, meaning no search was done.
I also added a method that returns if any search is applied on the table.
I need it because I dynamically change the pagination mode based on it.
I got tables with 1 million of records or more.
I don't want pagination when no search is done (execution query for the count is over 30secs). But as soon as a search is done, I can display the pagination.
class ArticleEntrepotListe extends Component implements Tables\Contracts\HasTable
{
    use Tables\Concerns\InteractsWithTable;

    use Tables\Concerns\InteractsWithTable {
        paginateTableQuery as protected parentPaginateTableQuery;
    }

    protected function paginateTableQuery(Builder $query): Paginator
    {
        if ($this->hasSearchApplied()) {
            return $this->parentPaginateTableQuery($query);
        }

        return $query->simplePaginate($this->getTableRecordsPerPage() == -1 ? $query->count() : $this->getTableRecordsPerPage());
    }

I've removed hasSearchApplied() as this isn't something we use internally and you can add it in a trait to your own project.

Thanks Dan !


How is train-error calculated when training on Spark?

How is train-error calculated when training on Spark?
I just find out that the train-error (binary classification) during the training (0.005 ) is smaller than the error rate after the training (0.05) on the same data.
The specific function I am using is trainWithDataFrame.

Problem found.
I set the missing default to 0 instead of NAN. My bad.


Missing documentation for minecraft:foil

Items support component "minecraft:foil" that gives them enchantment glint effect, but it's not documented.

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

ID: e43e50ff-ba33-036b-f395-e01f225d5e3e
Version Independent ID: 707cfc91-6885-cf1c-0a1a-65988a473516
Content: Item Documentation - Item Component List
Content Source: creator/Reference/Content/ItemReference/Examples/ItemComponentList.md
Product: gaming
GitHub Login: @mammerla
Microsoft Alias: v-bbortree

Good catch! minecraft:foil item component page has been added and will be visible after the next release. Thanks!


Add inline short-hand for glue:any role

In RMarkdown, they have a short-hand for inserting the values of r code inline into the document: `r somevariable`. Right now, we'd accomplish the same thing with {glue:}`somekey`  (after glueing it into the notebook).
I wonder if it would be helpful to think up a similar short-hand for variable insertion with MyST-NB. Some random ideas:

{{ somekey }}
`j somekey` (j for jupyter)
`g somekey` (g for glue)
{g}`somekey`

cc @stefanv who mentioned this earlier

Would this be using glue keys or actually variables? The former has a disadvantage compared to R because of referring to a different data abstraction. If the latter, would expressions be also OK?

Yes - I was going to say the same as @akhmerov - the R version really hits the transparency sweet-spot:
```{r}
# Some calculation
a <- 1
```

The value of `a` is `r a`.

This is so transparent that you can leave this markup in the student's notebook with the reasonable hope that the student will immediately see what is going on.
This isn't as true of the Glue syntax:
```{python}
from myst_nb import glue
# Some calculation
a = 1
glue('a', a)
```

The value of `a` is {glue:}`a`.

We need to first Glue and then paste, which requires explanation for someone who can see the markup.
Is it practical to make something similar to the inline r version, that has access by default to notebook variables, without explicit Gluing, and can evaluate code?

I agree that it would be much simpler if we found a way to let people insert variables into their documents that both:

Wasn't language-specific (glue() is a python function)
Didn't require extra code in the code cells that wasn't related to running analyses etc

I'm trying to wrap my head around how we'd technically be able to do this. Just spitballing some ideas here:

We run the notebook top-to-bottom as a part of the Sphinx parsing process. At that point we also have the content of the markdown cells
As we encounter glue syntax in the markdown, we assume that it points to a variable Perhaps we could parse the markdown for any "glue" directives or roles, and if one is found, call display() on that variable and store the result for injection into the markdown later (e.g. we can update the content of the role/directive so it points to a programmatically-generated key name)
All of this would apply only to same-page documents, I think we still need an explicit "store this key/val" step for pasting between pages

Running the notebook top-to-bottom before substituting inline expressions would use the latest available values of the variables that were mutated, once again deviating from the r abstraction (and the notebook abstraction itself).
Is it correct that the main design limitation here is the need to produce a jupyter notebook that has the same execution outcome? (otherwise inline executable code would be sufficient, it seems)

note that in my above comment we don't have to run the notebook top-to-bottom first, if we are able to run it cell-by-cell and inspect the markdown in between as we do so. However I think doing this would require a fairly large change in how we execute notebooks since (to my knowledge) no other jupyter infrastructure supports this

I must admit I started typing and wiped everything several times because I wasn't sure about responsibilities and guarantees of each project. Let me see if I got it right.

MyST-NB is built to use jupyter-cache for execution of notebooks?
Is the output of execution another notebook?
What guarantees should that other notebooks satisfy? This, I think is the most interesting part. Broadly speaking "the notebook should be similar to just being directly run", but is that done now? Is that a design goal?
Exactly how compatible does MyST-NB aim to be with jupyter? Rendering MyST is clearly not something Jupyter is able to do right now?

I'm going to assume that the answers are "yes", "yes", "mostly", and "mostly". If that is the case, I imagine a reasonable compromise would be to treat inline executable code as code for the purposes of what gets passed to the jupyter-cache, and store the outputs as markdown cell attachments.
This has a drawback that MyST-NB would potentially produce a different execution result than Jupyter if someone glues in a mutation in their markdown code. On the other hand, hopefully most authors would be reasonable enough to not do this.

The notebook also doesn't have to implement all the MyST features.  If I saw markup such as
the value of N is =N=

in a notebook, I'd know what it means, and I'd presume it is meant for some publishing tool to render.  It feels restrictive to tie the format to what the notebook can currently render, instead of thinking about what authors would want ideally.

It feels restrictive to tie the format to what the notebook can currently render, instead of thinking about what authors would want ideally.

I would point out this is the direct opposite of what a lot of people/authors have been requesting. They want the notebook to basically fully render in the notebook.

if we are able to run it cell-by-cell and inspect the markdown in between as we do so

No we don't. That goes against the whole design philosophy of jupyter-cache, it doesn't even store the markdown. The whole point of it is that notebooks only need to be re-executed when code changes, not markdown, so you are not having to constantly re-execute the notebook, when you are only changing the text.
I'm surprised @choldgraf and @akhmerov don't remember this, since we had quite lengthy conversations about it lol 😉

Is it practical to make something similar to the inline r version, that has access by default to notebook variables, without explicit Gluing, and can evaluate code?

Unfortunately no, this is just not going to happen; at least in the near term. The only reason that RMarkdown can do this is that they have built bespoke execution engines. Also, as discussed in their documentation, https://bookdown.org/yihui/rmarkdown/language-engines.html, this feature is only available for r, python and julia languages.

It feels restrictive to tie the format to what the notebook can currently render, instead of thinking about what authors would want ideally.

I would point out this is the direct opposite of what a lot of people/authors have been requesting. They want the notebook to basically fully render in the notebook.

Sure, but the question is whether the notebook should be able to do it already, or whether the notebook could learn to do it in the future.

I think "in the future" are the operative words there lol. I would push for changes in those packages first. Then we can re-assess when/if these render capabilities are available in the notebook.

No we don't. That goes against the whole design philosophy of jupyter-cache, it doesn't even store the markdown. The whole point of it is that notebooks only need to be re-executed when code changes, not markdown, so you are not having to constantly re-execute the notebook, when you are only changing the text.
I'm surprised @choldgraf and @akhmerov don't remember this, since we had quite lengthy conversations about it lol wink

I remember! (Although vaguely, since it happened in what feels like one of the previous epochs).
Still there's no requirement that jupyter-cache gets as input from MyST-NB the same notebook what MyST-NB sees. It's up to MyST-NB to inject a code cell per inline executable code role into what it sends to jupyter-cache.

it happened in what feels like one of the previous epochs

Before the apocalypse lol

to inject a code cell per inline executable code role

How do you structure a notebook to have inline executables?
e.g. what if there is an inline executable in the middle of a list
- abc `r x` efg

Here does this translate to a notebook?
I'm not saying it can't be done, but it would require an entire re-write of the current code; just to incorporate (at least at this stage) a "nice to have" feature.

I think there are several notebooks in question here. The code example you showed would be inside a markdown cell in the initial notebook, then x would be in a code cell for jupyter-cache.
For example I imagine MyST-NB could follow these steps:

parse the notebook
insert a code cell with contents x before the markdown cell when sending the notebook do jupyter-cache
take the jupyter-cache evaluation result
extract the outputs of the cells preceding the markdown cell in question
add them as attachments to the markdown cell
convert everything to sphinx AST.

Just a quick note here - I think we should set the context for this conversation as "wouldn't it be great if", rather than "let's implement this now". Sorry if I didn't make it clear before, but I agree w/ @chrisjsewell that this would require a lot of re-writing for how execution happens. I just want the conversation to be expansive and creative - but is very much a long-term kind of conversation

For example I imagine MyST-NB could follow these steps:

parse the notebook

Already at step (1) this in a divergence from what myst-nb currently does: the notebook doesn't get parsed until after it has been retrieved from jupyter-cache.

This has been a requested feature in Jupyter notebooks for many years - here's a recent thread that refers to previous discussions, probably here and here.  It's also a popular question on SO:

https://stackoverflow.com/questions/21808642/inline-python-in-markdown-with-ipython-notebook
https://stackoverflow.com/questions/21364102/expand-variables-in-markdown-cells-of-ipython-notebook
https://stackoverflow.com/questions/18878083/can-i-use-variables-on-an-ipython-notebook-markup-cell

Just a note that there was actually a "classic notebook" extension that did this: https://github.com/ipython-contrib/jupyter_contrib_nbextensions/tree/6af8e5e84e4746476c5b476b7e38f63d7abb2064/src/jupyter_contrib_nbextensions/nbextensions/python-markdown
I agree wholeheartedly that this would be an awesome feature within Jupyter, but we should be realistic that right now we don't have the connections to the JupyterLab world, nor the developer resources, to actually implement this. It's something we can advocate for and try to nudge in a direction, but would be non-trivial to figure out.

Another idea here - could we use notebook-level metadata to define variables that users wish to Glue into the notebook? In the context of jupyter-cache, I believe that the notebook metadata gets passed as well as the code cells (right?) and if so, then we could do something like:

Tell users to define the variables they want "glued" into the notebook with metadata, something like:
{"glue": ["var1", "var2"]}

After executing the notebook, Jupyter Cache checks the variables listed in the notebook metadata. If it finds a variable listed there, that also is defined at the end of execution, then it calls the kernel's "display" on it, and stores the results with the variable name as the key name (perhaps optionally, notebookname:variablename)

These results are also stored as either:

notebook-level metadata
A cell w/ a remove_cell tag that is appended to the bottom of the notebook

Then everything after this is the same as how glue currently works

That could potentially be

a way to disentangle the "glueing" code from the user's "doing stuff" code
a more reproducible behavior for users to understand (since right now they can do glue('mykey', a); a+=1; glue('mykey2', a) which might be confusing). Because each variable is only glued once after execution, it could be obvious what its value is
a way to be language agnostic (so users don't need to remember how to glue stuff in one language vs. another, they just need to know to put the right metadata in the notebook and a developer has to define how to call a display function once)


Adding utility to calculate max-pods for kubelet

Issue #, if available: N/A
Description of changes:
This adds a script that can be used to determine the Amazon EKS-recommended max-pods per node value, which depends upon several factors like instanceType, AWS VPC CNI version and the env variables set on the CNI DaemonSet like ENABLE_PREFIX_DELEGATION.
This script is written to be used in conjunction with the AWS VPC CNI plugin - https://github.com/aws/amazon-vpc-cni-k8s
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@suket22 how is this script supposed to be called? will there be another PR to replace the --max-pods calculation in bootstrap.sh to use this logic?

Also, would be handy if the +2 is configurable for clusters where we have >2 hostNetwork daemonsets configured, and would help explain what the +2 represents (I saw this explained in elsewhere but not here).

This script sets a ceiling for max pods to 110 for "low CPU" instance types and 250 for "high CPU" instance types - what is the AWS reference for those numbers?

This is not an exact science, but is based on our own internal testing. While the Kubernetes upstream project recommends no more than 110 pods per node, our testing has shown you can go higher on larger EC2 instance types. We’d recommend you run your own testing based on your applications, and you are free to override the kubelet settings and set a max pods value that meets your requirements. The important point is that with prefix assignment, IP addresses is no longer an artificial limitation to the number of pods you can run per node.

@adammw
We won't be updating the --max-pods calculation in bootstrap.sh. The bootstrap script is unable to determine which AWS CNI version is running on the cluster, and therefore cannot execute the max-pods-calculator script.
The way to think about this is if you're using self-managed nodegroups, you can pre-compute what the EKS-recommended maximum value for max-pods should be based on the CNI version you're running, your need for custom networking and prefix delegation by executing this script.
You can then update the UserData of your instances (via your EC2 LaunchTemplate if you use one) and set the bootstrap script as -
/etc/eks/bootstrap.sh <your-cluster> --use-max-pods false --kubelet-extra-args '--max-pods=<your_value>'


Fetcher obtains data, but data and error are undefined

Love the simplicity of this library. I’m having some trouble in the handoff of the data from the fetcher to useSWR.
My setup

Typescript
NextJS
Fetcher - GraphQL Client (fetch.ts)

require("fetch-cookie/node-fetch")(require("node-fetch"))
export default async function(query, variables, token) {
  const data = await graphql.request(query, variables)
  console.log("fetch.ts ", data)
  return data
}

SWRConfig

<SWRConfig value={{ fetcher: (query, variables, token) => fetch(query, variables, token) }}>

<SWRConfig value={{ fetcher: (query, variables, token) => fetch(query, variables, token).then(res => console.log(res)) }}>

Both setups return valid data.

useSWR

const { data, error } = useSWR<DataQueryType>([GraphQLQuery, props, token])

Data and error remains undefined despite the fetch response being successful on the server and fetch client.
Refreshing the page doesn’t appear to help either. It seems like the useSWR isn’t passing the result to the data object.
Happy to gather additional logs/details.

Just found #71. Will try and report back. In my case, I’m not using state but haven’t directly specified a type for the return data.
Might be a useful note to add to the docs.

Can you share an demo (maybe a codesandbox link) so I can reproduce? Thanks! 🙏

@jasonlor how are you going to use this library with NextJS if the latter relies on static getInitialProps (an entirely different approach to data fetching)?

@ivan-kleshnin Next.js provides getInitialProps but does not require you to use it (with Next.js 9, pages are automatically optimized between SSR and Static Prerendering).
useSWR is very compatible with Next.js, all of the examples use it!

@pacocoursey I mean people use NextJS mostly to get SSR. useSWR library does not support SSR at the moment. So NextJS + useSWR combination makes little sense to me.

@ivan-kleshnin SWR is mostly for client side data updates. For example a dashboard or any web apps with changing data.
That said, even if it is server side rendered, you’ll still have to tackle that problem. That’s why we have been using Next.js and SWR on zeit.co for more than 6 months.
Currently the community seems not to have a clear solution on Suspense especially with preloading, SSR, etc. and it’s still an experimental feature. But you can still use the getInitialProps way and here’s an example:
App.getInitialProps = async getInitialProps () {
  const data = await fetcher('/api/data')
  return { data }
}

function App (props) {
  const initialData = props.data
  const { data } = useSWR('/api/data', fetcher, { initialData })
  // note there’s this `initialData` option available

  return <div>{data}</div>
}

In this example, everything works the same just like a normal SSR Next.js app. But it’s also fully powered by SWR in the client side, which means the data can be dynamic now.
Sorry for not mentioning it in the documentation — we want to explore all the possibilities carefully and make sure it really works, before adding it as a new feature.

First time putting something into code sandbox. I hope this is right.
https://codesandbox.io/s/admiring-sun-8s0vy?fontsize=14&hidenavigation=1&theme=dark
SWRComponent, URQLComponent
I setup two components, one using SWR and one with URQL.
Both are pointed to the same graphql endpoint. For SWR, graphql-request lives in fetch.ts.
By default, the SWRComponent is disabled due to this error:
A cross-origin error was thrown. React doesn't have access to the actual error object in development. See https://fb.me/react-crossorigin-error for more information.
I have a console log outputting under App() that indicates that this might be from SWR.
console.log(fetch(query).then(res => Promise.resolve(res)));
This code, poorly typed, should illustrate and perhaps indicate the problem.

Fetch-client successfully returns data.
SWR has an error, potentially due CORS.

I hope this helps.

Hi @jasonlor! I forked your sandbox and fixed the issue: https://codesandbox.io/s/beautiful-thunder-7lylp
There're 2 problems, one is that you shouldn't pass {} to useSWR's args:
useSWR([query, {}, "bleh"])

Because SWR shallowly compares those args, just like the deps array in useEffect (explained here: https://github.com/zeit/swr#multiple-arguments).
And the other one is the extra .data here:
{data.data.country.name}

Hope that helps!

Let me know if you have other questions  🙏


I noticed that

Can you clarify "Conferenced in but are muted" notification format? The description implies that the Notification is a chat, but says recipients "can listen (but not participate)."  Thanks!

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

ID: 66544770-c56b-ad8a-5b67-9e20cbc945f1
Version Independent ID: d69667d5-d486-4b99-5e07-edb2fda7f5c7
Content: Manage emergency calling policies in Microsoft Teams - Microsoft Teams
Content Source: Teams/manage-emergency-calling-policies.md
Service: msteams
GitHub Login: @LanaChin
Microsoft Alias: v-lanac

@Tricia911 Thank you for submitting feedback and contributing to the docs. We are currently investigating this.

Hello @Tricia911 ,
when  "Conferenced in but are muted" is chosen:

someone make an emergency call
the people you specified will receive a notification about this call via Teams chat
Notification contains the link to the call, so they can join the call, but their microphones will be muted during the call (so they can only listen).
Thank you

@Tricia911 Thank you for submitting feedback. We understand that this issue has been resolved.
Please feel free to re-open this issue if there is a specific area of the docs that we can improve or make better. Thank you.


Support using cached builds

Once building is part of the Arbiter API, we could offer some generalized hooks that support looking up and reusing cached builds (instead of repeating a build from scratch).
For example, Carthage supports downloading pre-built frameworks uploaded with GitHub releases.
We probably don't need to get that specific, but we can add a generic API that allows the Arbiter client to install a cached build before we actually trigger our build step.

Not sure how to generalize this. If we figure it out, we can return to this idea.


Server error, cannot access local variable 'req_len' where it is not associated with a value

大佬们这个怎么解决啊

升级


Question: Can the bitrate of the songs downloaded be over 192kbs

I've noticed that all the songs i've downloaded so far are 192kbs, is it possible to get them at maybe 256 or 320 ?

No, the download bitrate cannot be improved. Also see #499, #137 and #52.


How do I get started with coding a custom main menu?

What is your question?
Title. Any templates out there to get me started?

for templates, there’s like a lot of options. Examples are like the menu options being on the side, more images in the menu, etc. you can find examples online


Is Possibility to sync signature into file

Hi,
Is it possible to sync signature into file instead of directly in messages?
Kind regards

Hi,
Do you want to have a file that contains the code of your signature? I think it should be technically possible, although it may require very broad permissions (https://support.mozilla.org/en-US/kb/permission-request-messages-thunderbird-extensions - usually extensions shouldn't have free access to your files, and the only one granular permission is to access downloaded files). I don't feel like doing it. What do you need it for? Maybe we could find an easier way to address your use case?

OK I get it.
Thunderbird is my main application for emails.
I have third party applications that can send emails and that need to contain my signature. These applications like Thunderbird can support a signature via a file.
I thought I could synchronize the signature via this extension to share it with the other applications.
I'll see if it's possible to do it differently.
Thanks for the quick answer.
Sincerely

Hi,
yeah, unfortunately, I think it could be a little tricky to integrate it into this add-on. This is a special case; but if you're familiar with JavaScript basics, maybe you could write a simple add-on, that would just save a current signature to the file, and use it locally. Used in parallel with my add-on, it should work, possibly with some delay.
I think that from Thunderbird API you just need to use accounts and identities objects, and then save the signature in a file.

thank you for the research leads.

No problem, hope you solve your case!

Just had a thought - so the addon itself never creates any sort of files locally, anywhere? Not even temporary files?
Just curious, not making any sort of accusation or trying to start a fight....

Exactly, the add-on is using just the local storage - this is the feature from Thunderbird (and any browser). The Thunderbird is responsible for storing it (and isolating from other add-ons). I've never verified what technology is used under the hood, but I think it's most probably SQLite database. But the add-on has no direct access to files, this is fully managed by the MailExtension API. I use this script to save data: https://github.com/thundernest/addon-developer-support/blob/09e1c5bb499f720659a52f2ce07b61339f7dc6b7/scripts/preferences/preferences.js
In the first version, the add-on needed so-called "full access" permission because the signature access wasn't available through the official API. As soon as signatures were incorporated into the official API, the add-on was updated to use it - and permission scope was limited to only necessary ones. Thanks to this, the Thunderbird ensures that it never access anything more than account identities (which handles signatures) and data from Google websites (for OAuth authorization and Gmail API access). Any try to access something more would be blocked.

Ahh, therein lay my error - not direct access to local files from the addon itself, only through Thunderbird.
Thanks for the explanation!


Support Java Script execution engine for web pages

Are there any thoughts on this?
Are we planning to use Splash [1] or Selenium [2] ?

My opinion:
Priority(basic things) > Priority(fancy features)
My definition of basics:

Horizontally scalable: distributes the workload to all nodes in the cluster
Open source: licence is Apache Licence 2.0 or atleast compatible with it.
Portable: the only assumption about the environment is a standard Java Runtime and nothing more.

In other words, I love to get some JS engine working within JVM. Thus we can distribute the load to all the nodes in the cluster. If that is not possible, the second option would be to proxy to an HTTP API (assuming the JS execution is still distributable behind the proxy).
@smadha  Come on board, research about the options, and get us some working piece of code. Thanks!

#37

resolved by #37


cmake: Don’t add -lobjc in linux builds

Fixes a linux build error.
It doesn’t make sense to let add HAS_LIBSWIFT to add_swift_host_library(). This was added to work around a linker bug (d22b348adb21f92a528b5586928c79adec021979). Instead do the workaround in libSwiftScan/CMakeLists.txt.

@swift-ci smoke test


Investigate if RefreshToken functionality is optimal

Description
We have RefreshToken functionality based on what was previously done in team-studio that there's been some concerns about if it's properly implemented.
How it's working:

User is logged out after 30 min of inactivity even though they're active on another tab.
RefreshToken is refreshed after every 10 minutes which grants user a refresh token valid for 30 min.

Tasks:

[x] Go to RefreshToken.tsx and Investigate if active tab is considered when refreshing the token (seems like it isn't and it should consider if the user has been active in the exact tab).

[ ] Improve functionality if optimizations can be made.

Additional Information
No response
Tasks
No response
Acceptance Criterias
No response

Results after investigation:
How it's working:

User is logged out after 30 min of inactivity even though they're active in another tab (that was the main scope to find out in this task).
RefreshToken is refreshed after every 10 minutes which grants user a refresh token valid for 30 min.


Pass Console parameters passed to logging.RichHandler constructor

Type of changes

[ ] Bug fix
[X] New feature
[ ] Documentation / docstrings
[ ] Tests
[ ] Other

Checklist

[X] I've run the latest black with default args on new code.
[X] I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
[X] I've added tests for new code.
[X] I accept that @willmcgugan may be pedantic in the code review.

Description
Enable to pass Console constructor parameters to the RichHandler constructor.
This way, through the logging.config.dictConfig, without dependency to the Rich project, a module can redirect its logs the the stderr with
{
  "handlers": {
    "rich": {
      "class": "rich.logging.RichHandler",
      "console": "cfg://stderr_console"
    }
  },
  "stderr_console": {
    "stderr": true
  }
}

Sorry, not keen on this change. A union of a Console and a dict of Any is an invitation for typing errors.
I'd suggest exposing an entrypoint for the handler in your own project.


Remove unused optimizeCb case

Currently unneeded

:pensive: yeah, I guess it is unused, but making this function partial feels like a bug waiting to happen.

I've added a clarifying comment about this intentional omission: https://github.com/jashkenas/underscore/commit/acf826fa42ba65d42c7e5cccd78ea3a7876c8032


[Feature Request] expects table components to add scrollPosition to props

Existing Component
是
Component Name
el-table
Description
需求：具有横向滚动的表格，希望可以通过props直接滚动到最后面。调试发现表格组件有个 State： scrollPosition，希望暴露到props里给开发者使用。

I do not understand Chinese, I am using google translate, so pardon me if my understanding is incorrect.

Well scrollToPosition will be really a nice feature, in an multiuser situation when a user is in 4th page and deletes an record, we refresh the data again, but the user is taken back to the first page. Instead it would be nice if I can bring him back to same page/position either by an method or a prop

And when switch tabs, the scroll positions of inner tables will be lost


[Bug] I18N support problem while importing XRC file [sf#28]

Reported by godoy on 2003-05-26 12:11 UTC
While converting an XRC file with xrc2wxg.py, it ommits
the encoding especification preventing wxGlade from
loading the generated wxg file.
IMHO, just copying the encoding from the original XRC
file is the correct solution (if the user didn't
specify it there, it is his fault).

Commented by agriggio on 2003-05-27 09:25 UTC
Logged In: YES
user_id=375945
Hello, the problem you are describing seems actually to be
two: the first (i.e. impossible to load certain files)
should be fixed on CVS, while the second (writing the proper
encoding) isn't, but I'll put it on my TODO.
Thanks for the report,
Alberto


Fix logic error in Linux active runtime search

When XDG_CONFIG_DIRS was unset, the fallback directories were ignored, resulting in /etc/xdg not being searched by default.
In retrospect I'm not sure what I was thinking with the original GetXDGEnv, that was way too complicated and it bit us.

So this in practice results in the following paths on my system being searched:

/home/ryan/.config/openxr/1/active_runtime.json
/etc/xdg/openxr/1/active_runtime.json
/usr/local/etc/openxr/1/active_runtime.json
/etc/openxr/1/active_runtime.json

which seems reasonable enough - the etc/xdg dir is newer standard so it comes ahead of the older stuff, and /usr/local/etc (or whatever your install prefix is) precedes /etc


Add configurable comma between attributes on multiple lines to PrinterContext

This allows the user to determine if commas are added between attributes on separate lines. This doesn't change the default but allows the user to disable commas via the with_no_node_mult_attr_s_l_comma builder method.

Great! Thanks for the pr!


YAML anchors with hyphens are not correctly highlighted

Information

Language: YAML
Plugins: none

Description
In a YAML document containing anchors with hyphens only the part of the anchor up to the first hyphen is highlighted.
Code snippet

The code being highlighted incorrectly.
x-foo-bar: &foo-bar 13

@RunDevelopment you're definitely fast! :)
However, #2215 will break some other cases. Therefore I propose to use #2217 instead.


Concurrent client requests

Feature Request
Motivation
It is common to have a need to do concurrent client requests, but it seems that Grpc client dispatcher is implemented to handle one call at a time, so the following example does not work by design:
use futures::join;

pub mod hello_world {
    tonic::include_proto!("helloworld");
}

use hello_world::{client::GreeterClient, HelloRequest};

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut client = GreeterClient::connect("http://[::1]:50051")?;

    let request1 = tonic::Request::new(HelloRequest { name: "hello".into() });
    let res1 = client.say_hello(request1);

    let request2 = tonic::Request::new(HelloRequest { name: "hello".into() });
    let res2 = client.say_hello(request2);

    println!("RESPONSES={:?}", join!(res1, res2));

    Ok(())
}

error[E0499]: cannot borrow `client` as mutable more than once at a time
  --> tonic-examples/src/helloworld/client.rs:17:16
   |
14 |     let res1 = client.say_hello(request1);
   |                ------ first mutable borrow occurs here
...
17 |     let res2 = client.say_hello(request2);
   |                ^^^^^^ second mutable borrow occurs here
18 |
19 |     println!("RESPONSES={:?}", join!(res1, res2));
   |                                      ---- first borrow later used here

Proposal
It seems that there is a need for multiplexing and a pool of clients.

@frol I believe that the .clone() operation on a tonic client should be relatively lightweight (just a handle to the underlying channel). You should be able to dispatch a request in parallel by cloning the client before executing its method.

@zackangelo is correct!
So pretty much this comes down to how tower handles back pressure. Internally we need a &mut self within the future to keep checking if our inner service is ready to accept the next request. To do this we need to borrow the service exclusively because only one access to that service can submit the request when its ready. To fix this internally we use a buffer channel to allow you to multiplex this concept of polling ready and handling back pressure. Hopefully this explains it a bit better.

Thank you for the thorough explanation!
Just to confirm, I was able to use .clone() just fine:
use futures::join;

pub mod hello_world {
    tonic::include_proto!("helloworld");
}

use hello_world::{client::GreeterClient, HelloRequest};

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let main_client = GreeterClient::connect("http://[::1]:50051")?;

    let mut client = main_client.clone();
    let res1 = client.say_hello(tonic::Request::new(HelloRequest { name: "hello1".into() }));

    let mut client = main_client.clone();
    let res2 = client.say_hello(tonic::Request::new(HelloRequest { name: "hello2".into() }));

    println!("RESPONSES={:?}", join!(res1, res2));

    Ok(())
}

Could you help me with a solution to fetch a list of resources concurrently (e.g. I have a vector of tonic::Requests)? I have tried to use futures::future::join_all:
pub mod hello_world {
    tonic::include_proto!("helloworld");
}

use hello_world::{client::GreeterClient, HelloRequest};

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let main_client = GreeterClient::connect("http://[::1]:50051")?;

    let mut client = main_client.clone();
    let res1 = client.say_hello(tonic::Request::new(HelloRequest { name: "hello1".into() }));

    let mut client = main_client.clone();
    let res2 = client.say_hello(tonic::Request::new(HelloRequest { name: "hello2".into() }));

    println!("RESPONSES={:?}", futures::future::join_all(vec![res1, res2]).await);

    Ok(())
}

error[E0597]: `client` does not live long enough
 --> tonic-examples/src/helloworld/client.rs:17:16
  |
17|     let res2 = client.say_hello(tonic::Request::new(HelloRequest { name: "hello2".into() }));
  |                ^^^^^^ borrowed value does not live long enough
...
22| }
  | -
  | |
  | `client` dropped here while still borrowed
  | borrow might be used here, when `res1` is dropped and runs the destructor for type `impl core::future::future::Future`

And even if that would work, I would still need to store the clients somewhere in a list, right?

@frol this works:
#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let main_client = GreeterClient::connect("http://[::1]:50051")?;

    let mut client1 = main_client.clone();
    let mut client2 = main_client.clone();

    let res1 = client1.say_hello(tonic::Request::new(HelloRequest {
        name: "hello".into(),
    }));

    let res2 = client2.say_hello(tonic::Request::new(HelloRequest {
        name: "world".into(),
    }));

    let responses = futures::future::try_join_all(vec![res1, res2]).await?;

    println!("RESPONSE={:?}", responses);

    Ok(())
}

Super-helpful... thank you.
If it was in a loop?  I'm generating the req/resp from the protobufs, and tonic::response::Response - I cannot clone it.
This definitely inspired me to test the streaming approach, which works swimmingly.  But, a batch of requests and handling their responses, does this require an Arc and pinning around the data-structure?

@blittable see https://github.com/hyperium/tonic/issues/44#issuecomment-543829569 for a possible solution

I think we can close this, feel free to reopen if you have any more questions :)

Rust and tonic noob here, but if client.clone() is cheap and required frequently, wouldn't it be easier if client types had the Copy trait and client calls would take self by value instead of mutable reference?

@ettersi Copy is for types that can be copied by simplying copying their byte representation, such as i32. Two i32s are identical if the contain the same bytes. Whereas something like Box<String> is different. You cannot get a clone of the box by just copying the boxes bytes on the stack. There is memory on the heap to deal with. So Copy doesn't simply mean "cheap to clone".

Right, I forgot that it has to be a bitwise copy. Sorry about the spam, then.

Quick question, given that the Clone is lightweight, why does the client need &mut self instead of &self ?

@gabrik see previous info https://github.com/hyperium/tonic/issues/33#issuecomment-538154015


Fix #434 (subnet sizes != /16)

This should fix #434. I took the freedom to simplify the command you proposed a bit. I also removed private_network_test_ip, which had the same problem with subnet size. My IDE ran shfmt on the .sh-files; I kept the reformatting, if you don't mind.
I also tried running e2e-tests (see GH-437), but stopped the execution, when tests where spamming dozens of errors quickly (image 'fedora-38' not found). I also had random connection issues after server creation. I did not investigate further at this point. I don't think they were related to my changes.
Random connection problems:
Error creating instance: timeout after 00:01:00
Instance creation for test-e9acedda-master1 failed. Try rerunning the create command.

Aborted test results:
config-sshport-image.yaml  IMAGE=alma-8 SSHPORT=22         error    done      test-15c94339
config-sshport-image.yaml  IMAGE=alma-8 SSHPORT=222        error    done      test-c59dc574
config-sshport-image.yaml  IMAGE=alma-9 SSHPORT=22         ok       done      test-3a378dbe
config-sshport-image.yaml  IMAGE=alma-9 SSHPORT=222        error    done      test-e9acedda
config-sshport-image.yaml  IMAGE=debian-11 SSHPORT=22      ok       done      test-77bf45fe
config-sshport-image.yaml  IMAGE=debian-11 SSHPORT=222     ok       done      test-fe655f1c
config-sshport-image.yaml  IMAGE=debian-12 SSHPORT=222     ok       done      test-e16755b9
config-sshport-image.yaml  IMAGE=fedora-38 SSHPORT=222     pending  creating  test-06bdf688
config-sshport-image.yaml  IMAGE=rocky-8 SSHPORT=22        ok       done      test-34ca1517
config-sshport-image.yaml  IMAGE=rocky-8 SSHPORT=222       error    done      test-48a902f8
config-sshport-image.yaml  IMAGE=rocky-9 SSHPORT=22        ok       done      test-3c4c5d45
config-sshport-image.yaml  IMAGE=ubuntu-20.04 SSHPORT=222  ok       done      test-9b2fbf59
config-sshport-image.yaml  IMAGE=ubuntu-22.04 SSHPORT=222  ok       done      test-86176827
config-sshport-image.yaml  IMAGE=ubuntu-24.04 SSHPORT=222  ok       done      test-b7c132d6

Thanks for the PR! It's a bit delicate change so I will need to test it throughly when I have more time.

I very much agree with you. This should not be rushed.


feat(RingTheory): being finitely-presented is local

Thanks!
maintainer merge

bors merge

Thanks for the reviews!


Make datastore management object oriented

We have a growing list of data sources, with different internal structures, which is starting to tax the existing datastore.py module with a bunch of special cases. MSHA is all years in one file, EPA breaks down by month and by state, many of the data sources have more recent partial year information, etc. This seems like an ideal case for object oriented design -- with a general data source object that has different implementations of the same methods for each data source, so we can interact with all of them similarly, rather than need a ton of special cases.

Request: can we totally disable loading some datasets? It would make it way faster to test specific things if we don't have to load 2+ form 1 years every time. I also imagine some people would want to disable CEMS (this could even be the default), since it's so large.

Yeah, I wonder if want to set things up to bring in each data source independently, and to then determine which glue is required to connect the pairs of datasets that have come in.

See also #314 reported by @gschivley.

So, we're (aka @ptvirgo) in the process of re-writing the datastore management code, and splitting off the data acquisition into a different tool that will produce a versioned archive of the original data as it was available from the government sources, which gets updated whenever they change the data (as they seem to on a regular basis) which will facilitate reproducibility without requiring us to archive all the input files individually with any data publication. The plan is to have it run as a daily cron job that notices when files get changed, and automatically generates a new versioned deposit at Zenodo with a DOI etc. that can be cited directly, and accessed via their clean REST API, which will hopefully let more people build stuff that can use programmatic access to the raw data even if they aren't using PUDL.
In addition, the data as it comes from EIA, FERC, etc. often changes file names and directory structures (inside the ZIP files) a bit over the years, and we are wondering (@gschivley @karldw @joshdr83 @briannacote) whether it would be more useful to have the versioned raw data archive at Zenodo consist of exactly the same files that have been downloaded from EIA/FERC/etc. including the preservation of their inconsistent filenames and directory structures (ugh), or whether it would be better to standardize those names and directory structures (without touching the contents of the original files themselves) before archiving them.
Building this standardization logic into the tool that downloads and archives new versions of the data would mean that someone could easily make programmatic use of the Zenodo archive without needing to use our tooling, and without needing to re-implement the logic that deals with these inconsistencies.
However, it would also mean that the files aren't utterly pristine.
Which do y'all think would be the better option?

@zaneselvans, I don't have a strong opinion here. If you wanted to be sure absolutely no information was lost, you could add a log file that describes how each file was renamed.

That seems like a good compromise!
Joshua Rhodes
https://sites.google.com/site/joshdr83/
@joshdr83

On Oct 31, 2019, at 7:37 PM, Karl Dunkle Werner<EMAIL_ADDRESS>wrote:
@zaneselvans https://github.com/zaneselvans, I don't have a strong opinion here. If you wanted to be sure absolutely no information was lost, you could add a log file that describes how each file was renamed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/catalyst-cooperative/pudl/issues/182?email_source=notifications&email_token=AAZQFTG5W2BTZ6HFSWC5Q7LQROB6HA5CNFSM4FF6BU5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECZXQSQ#issuecomment-548632650, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQFTE2EJ5FCYYP35E45VLQROB6HANCNFSM4FF6BU5A.

If no one has a strong preference then:

Munging it first has a higher overall complexity which means:
You get to pay me more money to build something nobody really needs.

Which I'm mostly fine with, as long as everyone is clear that this is
what you're asking for.
On 10/31/19 9:54 PM, Joshua Rhodes wrote:

That seems like a good compromise!
Joshua Rhodes
https://sites.google.com/site/joshdr83/
@joshdr83

On Oct 31, 2019, at 7:37 PM, Karl Dunkle Werner
<EMAIL_ADDRESS>wrote:
@zaneselvans https://github.com/zaneselvans, I don't have a strong
opinion here. If you wanted to be sure absolutely no information was
lost, you could add a log file that describes how each file was renamed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/catalyst-cooperative/pudl/issues/182?email_source=notifications&email_token=AAZQFTG5W2BTZ6HFSWC5Q7LQROB6HA5CNFSM4FF6BU5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECZXQSQ#issuecomment-548632650,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAZQFTE2EJ5FCYYP35E45VLQROB6HANCNFSM4FF6BU5A.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/catalyst-cooperative/pudl/issues/182?email_source=notifications&email_token=ABYRCMZT3GLT4XAQPYBNSVTQROD5BA5CNFSM4FF6BU5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECZYIEY#issuecomment-548635667,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABYRCM4ROEAAFY7WVXLIWF3QROD5BANCNFSM4FF6BU5A.


Commit action doesn't work in SCM tree

This repo / Codespace: https://github.com/iterative/iterative.ai
https://user-images.githubusercontent.com/3659196/218896789-4d4c1d7d-cce7-4ec8-804c-6256661550a1.mov
More weird examples:
(after reloading the container, bad error message + we are not trying to repeat + it doesn't allow to do it again):
https://user-images.githubusercontent.com/3659196/218897342-b7a28527-b2db-457e-82bd-6522edfee1a0.mov
Finally, when it works, the message is "ERROR", but also a warning. Something about stage - it's all very scary and confusing (cc @dberenbaum )
https://user-images.githubusercontent.com/3659196/218898967-25226cfd-297d-482c-bdd4-9554949df4f9.mov

Make it p1 since it breaks the experience severely when we deal with blog.

What does dvc data status --granular --unchanged --json output under these circumstances?

@shcheklein, this should be fixed with the latest version. Please reopen if I have misunderstood or missed something 🙏🏻.