[SPARK-9632][SQL] [HOT-FIX] Fix build.

seems https://github.com/apache/spark/pull/7955 breaks the build.

Merged build triggered.

Merged build started.

I tested locally. Merging it to master and branch 1.5.

Test build #40062 has started for   PR 8001 at commit 6c257dd.

Test build #40062 has finished for   PR 8001 at commit 6c257dd.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
sealed trait LogisticRegressionTrainingSummary extends LogisticRegressionSummary
sealed trait LogisticRegressionSummary extends Serializable
final class SpecificMutableRow(val values: Array[MutableValue])
      final class SpecificRow extends $
trait BaseGenericInternalRow extends InternalRow
class GenericMutableRow(values: Array[Any]) extends MutableRow with BaseGenericInternalRow

Merged build finished. Test PASSed.


[PDI-17103]-Transformation Executor: Parameters: "Static Input value"field are not populated with Variable value defined in child transformation if Getting Parameters

-fixed

Build Completed
:x: This pull request has errors. They will need to be addressed before it can be accepted. See below for more details. Some links are also available below for further assistance in addressing those issues.
Build Commands
mvn -B -fn -f 'pom.xml' -pl 'ui,engine' -P '!assemblies' -Dsurefire.runOrder=alphabetical -Daudit -amd clean install

Cleanup Commands
mvn -B -f 'pom.xml' -pl 'ui,engine' -P '!assemblies' -amd build-helper:remove-project-artifact

Changed files
  engine/src/main/java/org/pentaho/di/trans/steps/transexecutor/TransExecutor.java
  ui/src/main/java/org/pentaho/di/ui/trans/steps/transexecutor/TransExecutorDialog.java

Newly Broken Tests:
org.pentaho.di.trans.steps.transexecutor.TransExecutorUnitTest.testExecuteTrans:
:red_circle: java.lang.ArrayIndexOutOfBoundsException
java.lang.ArrayIndexOutOfBoundsException: 0
	at org.pentaho.di.trans.steps.transexecutor.TransExecutorUnitTest.testExecuteTrans(TransExecutorUnitTest.java:434)

Unit Test Coverage
These statistics help you identify how your changes have affected the coverage of the following files. If a file is not in this list, then its coverage was not affected by your changes.

Build Completed
:x: This pull request has errors. They will need to be addressed before it can be accepted. See below for more details. Some links are also available below for further assistance in addressing those issues.
Build Commands
mvn -B -fn -f 'pom.xml' -pl 'ui,engine' -P '!assemblies' -Dsurefire.runOrder=alphabetical -Daudit -amd clean install

Cleanup Commands
mvn -B -f 'pom.xml' -pl 'ui,engine' -P '!assemblies' -amd build-helper:remove-project-artifact

Changed files
  engine/src/main/java/org/pentaho/di/trans/steps/transexecutor/TransExecutor.java
  ui/src/main/java/org/pentaho/di/ui/trans/steps/transexecutor/TransExecutorDialog.java

Newly Broken Tests:
org.pentaho.di.trans.steps.transexecutor.TransExecutorUnitTest.testExecuteTrans:
:red_circle: java.lang.ArrayIndexOutOfBoundsException
java.lang.ArrayIndexOutOfBoundsException: 0
	at org.pentaho.di.trans.steps.transexecutor.TransExecutorUnitTest.testExecuteTrans(TransExecutorUnitTest.java:434)

Unit Test Coverage
These statistics help you identify how your changes have affected the coverage of the following files. If a file is not in this list, then its coverage was not affected by your changes.

Build Completed
:fire: This pull request has some issues. It would be preferable to fix them in order for it to be just perfect. See below for more details. Some links are also available below for further assistance in addressing those issues.
Build Commands
mvn -B -fn -f 'pom.xml' -pl 'ui,engine' -P '!assemblies' -Dsurefire.runOrder=alphabetical -Daudit -amd clean install

Cleanup Commands
mvn -B -f 'pom.xml' -pl 'ui,engine' -P '!assemblies' -amd build-helper:remove-project-artifact

Changed files
  engine/src/main/java/org/pentaho/di/trans/steps/transexecutor/TransExecutor.java
  engine/src/test/java/org/pentaho/di/trans/steps/transexecutor/TransExecutorUnitTest.java
  ui/src/main/java/org/pentaho/di/ui/trans/steps/transexecutor/TransExecutorDialog.java

Unit Test Coverage
These statistics help you identify how your changes have affected the coverage of the following files. If a file is not in this list, then its coverage was not affected by your changes. To get some help interpreting these metrics, please refer to Jacoco's documentation.

:warning: Coverage Changes (click to expand)
org.pentaho.di.core.injection.bean.BeanInjector

Branch Change: -0.7% :small_red_triangle_down:
Complexity Change: -1.2% :small_red_triangle_down:

org.pentaho.di.job.entries.evalfilesmetrics.JobEntryEvalFilesMetrics

Branch Change: -0.4% :small_red_triangle_down:
Complexity Change: -0.5% :small_red_triangle_down:
Instruction Change: -0.1% :small_red_triangle_down:

org.pentaho.di.job.entries.waitforsql.JobEntryWaitForSQL

Branch Change: -0.8% :small_red_triangle_down:
Complexity Change: -1% :small_red_triangle_down:
Instruction Change: -0.1% :small_red_triangle_down:

org.pentaho.di.trans.steps.constant.ConstantMeta

Instruction Change: -0.1% :small_red_triangle_down:

org.pentaho.di.trans.steps.groupby.GroupByMeta

Branch Change: -1.1% :small_red_triangle_down:
Complexity Change: -1% :small_red_triangle_down:

org.pentaho.di.trans.steps.memgroupby.MemoryGroupByMeta

Branch Change: -2.2% :small_red_triangle_down:
Complexity Change: -2.5% :small_red_triangle_down:
Instruction Change: -0.3% :small_red_triangle_down:
Line Change: -0.9% :small_red_triangle_down:

org.pentaho.di.trans.steps.replacestring.ReplaceStringMeta

Instruction Change: -0.1% :small_red_triangle_down:

org.pentaho.di.trans.steps.transexecutor.TransExecutor

Branch Change: -0.1% :small_red_triangle_down:
Complexity Change: -1.1% :small_red_triangle_down:

org.pentaho.di.trans.steps.webservices.WebServiceMeta

Branch Change: -1.7% :small_red_triangle_down:
Complexity Change: -1.2% :small_red_triangle_down:
Instruction Change: -0.1% :small_red_triangle_down:

org.pentaho.di.trans.steps.salesforceupsert.SalesforceUpsertMeta

Instruction Change: -0.1% :small_red_triangle_down:

@mbatchelor please review the changes


point -h to --help flag

would be useful to point -h to --help
./devd.exe -h
devd.exe: error: unknown short flag '-h', try --help

OK, I can see how that could save people a second or two.


feat(Pagination): add disabled prop

Fixes #3275.

Codecov Report

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

@@           Coverage Diff           @@
##           master    #3295   +/-   ##
=======================================
  Coverage   99.92%   99.92%
=======================================
  Files         169      169
  Lines        2803     2803
=======================================
  Hits         2801     2801
  Misses          2        2

Impacted Files
Coverage Δ

src/addons/Pagination/PaginationItem.js
100% <ø> (ø)
:arrow_up:

src/addons/Pagination/Pagination.js
100% <ø> (ø)
:arrow_up:

Continue to review full report at Codecov.

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


Compatibility with Silex 1.3 and symfony/form 2.4+

Hi!
After updating to Silex 1.3 while using 2.4+ version of symfony the following error occurs:
Argument 12 passed to Gigablah\Silex\OAuth\Security\Firewall\OAuthAuthenticationListener::__construct() must be an instance of Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface, instance of Symfony\Component\Security\Csrf\CsrfTokenManager given, called in /vagrant/vendor/gigablah/silex-oauth/src/Gigablah/Silex/OAuth/OAuthServiceProvider.php on line 132

This is because of this change in Silex 1.3:
https://github.com/silexphp/Silex/blob/1.3/src/Silex/Provider/FormServiceProvider.php#L109
It looks like Silex 1.3 now returns a CsrfTokenManager instead of an object of CsrfProviderInterface, but only when using symfony/form 2.4+.
Any suggestions/ideas on how to fix this?
It looks like it might be hard for you to be compatible with the different variations here.

+1

+1

This pull request would solve the problem(-s). Why not merge it?
https://github.com/gigablah/silex-oauth/pull/24

+1 for compatibility

Tag 1.3.0 has been pushed which provides compatibility with Silex 1.3 (<2.0) and Symfony 2.4+.


Missing deprecation guide for session events

Please explain how to migrate away from using session events to call sessionAuthenticated and sessionInvalidated.
ember-simple-auth.events.session-service

Yes please. I'm trying to cleanup the deprecations in an app here and am not seeing my extended session's handleAuthentication or handleInvalidation  ever being called, while the older on events will fire as expected.


Update Guides structure

Remove redundant directory, consolidate Guides landing page, create landing page for Contributing.

I updated order to address conflicts, but set it so contributing is above testing. Since we only have 2 pages, I don't think we need to create a sub-directory at this time, but will keep in mind if we add more content later. It looks like it would be good to add sub-directories to the Howtos though; I'll start a separate PR for that.

Sorry for not responding sooner on this PR. Could you resolve the merge conflict on the files noted above?

The merge conflicts are too complex to resolve in the web UI (per the UI tooltip), but I don't have a local repository since I've only been working in the web UI. I'm going to close this one out and try smaller command-line changes to make these fixes. Apologies for the trouble!

Adding cla: yes label, as the PR author is a googler at the time of this PR was created.


Review error handling/rollback when recieve_packet errors (sends ack packet with error)

This counts as a fail for the contract and we would assume state is rolled back.
But we return what looks like as a success to the ibc callback (which is correct)
Let's ensure this doesn't leave half-written state in ibc-reflect and ibc-reflect-send.

I have been thinking about this a bit more and followed some general discussion on this issue at https://github.com/cosmos/ibc-go/issues/68
When reflecting more on this, I realized that even if a contract caches it's state internally and only commits writes on success, what if it return a message? And that message errors?
The only proper place to handle this would be in the Go app (wasmd). It should make a cached store and execute the task and all messages and all sub-messages and if any errors, then it reverts the local store cache (just inside the wasm handler, not for the ibc handler), and package this error into an "error" acknowledgement message, returning a success with error acknowledgement to the ibc handler.
The main issue is creating the error acknowledgement. There is a standardized (but not required) acknowledgement format that is used in ICS20: https://github.com/cosmos/ibc-go/blob/main/proto/ibc/core/channel/v1/channel.proto#L144-L157 (thanks to me). This is currently encoded as JSON in the ics20-1 protocol, but there was some discussion to make this protobuf in the future.
I would propose the following mapping in x/wasm (which needs to be well-documented). If a contract returns an error on OnRecvPacket, the Go keeper should revert local state changes, and check the value of the error string to determine the acknowledgement packet (for error):

If 0x..., parse this as hex data and set the acknowledgement response binary
If {...}, assume this is a custom JSON format and convert it directly to []byte as acknowledgement
Else: Embed it in the Acknowledgement error field and serialize it as JSON. Basically {"error": err.String()} except with string escaping.

Open question: should we handle serializing the standard Acknowledgement in protobuf format?
Note: if a protocol requires a specific error ack format and an error may be returned by a message, it must return them as submessages, and if the submessage returns error, the contract can map that to a different custom error format (eg. 0xabcd) before returning that.

When reflecting more on this, I realized that even if a contract caches it's state internally and only commits writes on success, what if it return a message? And that message errors?

This is a scenario that is possible now but pretty complex to handle in wasmd. IMHO the contract is fully responsible to process or orchestrate the IBC data. We can simply remove Messages from the response and let the contract do everything with Submessages to be fully in charge and enforce the right behaviour. The ACK packet format is app level as well.

If Acknowledgement is a binary result or a string error, shouldn't we have an
pub enum Acknowledgement {
    /// The success case
    /// https://github.com/cosmos/ibc-go/blob/main/proto/ibc/core/channel/v1/channel.proto#L154
    Result(Binary),
    /// The error case
    /// https://github.com/cosmos/ibc-go/blob/main/proto/ibc/core/channel/v1/channel.proto#L155
    Error(String),
}

and use it instead of Binary in both IbcAcknowledgement::acknowledgement and IbcReceiveResponse::acknowledgement?

That channel acknowledgement is a recommended format for acknowledgements, but not all protocols must use it. (They went overboard on ultra-flexibility in IBC as to make it hard to use). Further, it is unspecified if this is encoded as JSON or Protobuf if it is used (6 months ago they said protobuf, implementation is JSON, this may have something to do with the event system).

Oh, this is MUCH more complex to handle in a contract. With submessages, I already handle the wrapping on the store and reverting if needed. Reverting state in execute is possible. But keeping that in some "to be committed" state and then only committing it after all submessages have returned success is an engineering nightmare.

Unless I understood the issue wrong it does belong to the app level and moving it to wasmd limits the flexibility that dynamic ibc gives us for custom IBC apps. Wasmd must not know anything about the data packages nor do assumptions what an error is. In my understanding the acknowledgement format is purely app specific although a recommended format exists.
Submessages do not alter state on error. So the contract has to keep track of submessages sent and their responses. It can abort and return an IBC ack error on the first issue or wait for the last submessage to succeed for the final OK ack. But also this is app dependent. Maybe there are scenarios where even a failing submessage does not cause an IBC ack error or different ack states exists....
Maybe we can have some contract framework that helps with the complexity on that level at some point.

Look at this (common) case:

Contract processes incoming message, updates some internal bookkeeping.
Contract returns submessage to transfer some tokens, as well as the acknowledgement
Contracts gets an error from submessage and wants to convert it to a app-specific acknowledgement format.

This is hard because:
A. How do I revert the state changes from (1) in another call to the contract?
B. Since I returned a (success) acknowledgement in step (2), there is no clear way to change it with submessages.
My approach which says, contract returns error in (1) or (3) on failure and error string encodes the (failed) acknowledgement packet makes use of most of the plumbing we already have. We error on any sub-call that returns errors, and just a simple cache wrap and error serialization will let us customize the ack from a failed sub-call.

I would note that the IBC team plans to support OnRecvPacket callbacks returning error to the IBC handler and manage this state reversion there (in a future IBC version). Meaning, we can be a step ahead with the contract interface using some glue code in tgrade, and then just remove most of the glue code and base on the Cosmos SDK in a future version.
I'm not sure how they plan to encode the acknowledgement message on failure, but probably some Go interface on the error.

Please see https://github.com/cosmos/ibc-go/pull/107 for the new ibc-go approach to revert on error.
From 0.43 on, this will be taken care of in the sdk. For now, we have to handle it in wasmd. But we should take a contract interface that works for the future

I propose a different mechanism to re-write the "error acknoweldgement" (rather than some ad-hoc encoding format in an error string). I would appreciate a review of the description here https://github.com/CosmWasm/cosmwasm/pull/885
Once we have agreement on the approach, I will implement it and close this issue.

Look at this (common) case:
1. Contract processes incoming message, updates some internal bookkeeping.

2. Contract returns submessage to transfer some tokens, as well as the acknowledgement

3. Contracts gets an error from submessage and wants to convert it to a app-specific acknowledgement format.

This is hard because:
A. How do I revert the state changes from (1) in another call to the contract?
B. Since I returned a (success) acknowledgement in step (2), there is no clear way to change it with submessages.

Isn't the problem in this use case that the acknowledgement needs to be created in the entry point ibc_packet_receive but its value is not known yet. Instead it should be created in the reply endpoint?
So what bites us here is no syncronous calls out and no promise system that would allow us to continue processing ibc_packet_receive when we gathered all information.

@webmaster128 can you please review https://github.com/CosmWasm/cosmwasm/pull/885 where I propose a solution to this? It allows easy extensibility of how to encode the "error acknowledgement" regardless of where the error came from (contract, message, submessage, message of message...).
It does assume that all information for a "success acknowledgement" is available at the time of the original contract execution and doesn't depend on any submessages. Let me try to think of a real usecase for that before worrying on a design of that, but it would be good to get feedback on the "error acknowledgement" part and if it looks good and there is need, this can inform the design of the "delayed success acknowledgement" issue

Okay, on reflection, the success case is also important.
In the (vague) ICS27 spec, they define a packet to register a new account, the acknowledgement of that packet should contain the address of the new account.
If we implemented this in CosmWasm, the contract would have to return a submessage of WasmMsg::Instantiate{} and only get the proper address for the response in reply.
I would propose a similar method to #885. If a contract needs to do this delayed, it can expose a ibc_receive_delayed_acknowledgement entry point (or different name). If present, this is called after all messages and submessages have been processed. And is provided the original ack bytes from the ibc_packet_recv call. If you don't need this delay, don't expose this and it works as normal. If you do expose it, you can write the answer to storage during reply, and then read it (and delete it) in this callback when you return it.
Unless there are major issues, I will refine this issue and add it to #885


Reduce Metrics/CyclomaticComplexity Max to 7

Today is a public holiday, and so I had time on my hands to do some clean up.
Through a lot of method extractions and the introduction of some inner classes, this change reduces cyclomatic complexity to 7, and takes a chunk out of perceived complexity and others.

Nice, @Drenmi! In case you're planning to continue with similar PRs, I guess it's good to have some coordination. In other words, I'm on MethodLength -> 15. :)

@lumeet: I will most probably work on bugfixes and features after this. I'll ask you if I plan to get back to housekeeping. 😀

Impressive work!

I have to give most of the credit to RuboCop. Having the TODO-file is a huge motivator to clean things up, as you can just lower the number one "click" at a time. 😀


CLOUDSTACK-8800 : Improved the listVirtualMachines API call to include memory utilization information for a VM

for xenserver,hyperv and for vmware.

LGTM (though not tested in an environment), anyone wants to review @wido @remibergsma @wilderrodrigues ?

I see tests only for LibrvirtResource. They are missing for StatsCollector, CitrixResourceBase, VmwareResource

@maneesha-p Why not for KVM?

@remibergsma @maneesha-p Indeed. We have the information, so why not KVM? That would cover allmost all cases.

@wido @remibergsma I am sorry I changed it for kvm but updated the description wrong.

@maneesha-p please rebase against latest branch and resubmit

@bhaisaab @wido @remibergsma Plz review.

This looks good to me. Hard to test though :(

@remibergsma @wido @bhaisaab  2 LGTMs can it be merged?

Tested Manually. memorykbs, memoryintfreekbs,memorytargetkbs are listed correctly.
LGTM.
<?xml version="1.0" encoding="UTF-8"?><listvirtualmachinesresponse cloud-stack-version="4.9.0-SNAPSHOT"><count>1</count><virtualmachine><id>a574e9d1-8057-4195-a8b3-2117e9059652</id><name>VM-a574e9d1-8057-4195-a8b3-2117e9059652</name><displayname>VM-a574e9d1-8057-4195-a8b3-2117e9059652</displayname><account>admin</account><userid>ee621684-952a-11e5-abd2-d4ae52cb9a54</userid><username>admin</username><domainid>ee5bfd6f-952a-11e5-abd2-d4ae52cb9a54</domainid><domain>ROOT</domain><created>2015-11-27T23:30:46+0530</created><state>Running</state><haenable>false</haenable><zoneid>b5503a30-f55b-4889-94c2-32a73c6b4166</zoneid><zonename>KK</zonename><hostid>57382ec5-87e1-4c0b-9df1-02b2cba04971</hostid><hostname>Rack3Pod1Host44</hostname><templateid>ee3b1574-952a-11e5-abd2-d4ae52cb9a54</templateid><templatename>CentOS 5.6(64-bit) no GUI (XenServer)</templatename><templatedisplaytext>CentOS 5.6(64-bit) no GUI (XenServer)</templatedisplaytext><passwordenabled>false</passwordenabled><serviceofferingid>1d88488d-6651-4a63-b865-a3a5f4115dd6</serviceofferingid><serviceofferingname>custom</serviceofferingname><cpunumber>4</cpunumber><cpuspeed>512</cpuspeed><memory>128</memory><cpuused>0.04%</cpuused><networkkbsread>0</networkkbsread><networkkbswrite>0</networkkbswrite><diskkbsread>0</diskkbsread><diskkbswrite>11</diskkbswrite><memorykbs>131072</memorykbs><memoryintfreekbs>15</memoryintfreekbs><memorytargetkbs>131072</memorytargetkbs><diskioread>0</diskioread><diskiowrite>0</diskiowrite><guestosid>ee4c163d-952a-11e5-abd2-d4ae52cb9a54</guestosid><rootdeviceid>0</rootdeviceid><rootdevicetype>ROOT</rootdevicetype><nic><id>66347c35-f51f-4527-bb59-50a976d90927</id><networkid>d97370e1-7fb3-44be-8af5-9515c1d624a2</networkid><networkname>n1</networkname><netmask><IP_ADDRESS></netmask><gateway><IP_ADDRESS></gateway><ipaddress><IP_ADDRESS></ipaddress><isolationuri>vlan://514</isolationuri><broadcasturi>vlan://514</broadcasturi><traffictype>Guest</traffictype><type>Isolated</type><isdefault>true</isdefault><macaddress>02:00:66:75:00:01</macaddress></nic><hypervisor>XenServer</hypervisor><instancename>i-2-3-VM</instancename><details>{hypervisortoolsversion=xenserver56}</details><displayvm>true</displayvm><isdynamicallyscalable>true</isdynamicallyscalable><ostypeid>142</ostypeid></virtualmachine></listvirtualmachinesresponse>

I see this was merged into master this morning.  Hmmm...

should we revert this merge in favor of fixing these issues?

@swill you can create a PR if you want.

@ustcweizhou the point is more that there are things in this PR that I am not comfortable with and as the 4.9 RM, I need to be comfortable with the code that goes into master.

@swill, I understand your position; as you, I also think that there was room for improvements in this PR (especially to avoid possible runtime exception). However, reverting a PR is always something complicated. I believe we should wait to hear feedback from others, before taking any decision.

@rafaelweingartner I don't mind waiting a bit, but runtime exceptions need to be taken seriously.  As the 4.9 RM, it is my responsibility to make sure this type of stuff is sorted out BEFORE it gets merged into master.  I welcome others to give feedback, but I would recommend that we do a git revert on this merge and at the very least code defensively around the runtime exceptions.

@swill Please do proceed with revert, I totally agree.
git revert -m 1 dc0ba6bd1a774d3ff4bc4a4dcc00e1434ab1f6e3 will do. Let me know if you need help.

@swill I would not allow any other merge before this is fixed or reverted!

@swill @remibergsma @DaanHoogland PR was open since Sep 2015. Review from @swill came after the PR was merged on Mar 17 2016. By then, there were code reviews and tests done with 3 LGTMs. I merged the PR as per the current process.
Unfortunately all reviewers missed the issued mentioned by @swill
Thanks to @rafaelweingartner https://github.com/apache/cloudstack/pull/1444 addresses the runtime exception issues.
Unless there are issues with PR 1444, I do not see a need to revert this merge. Can we all please review PR 1444 and close this issue?

@kishankavala you are formally correct and we had not closed master for release so I don't blame you for the merge. The issue as brought up by @swill is not to blame anybody for the merge but to preserve the quality of master before doing so. I am just saying that we should close master now and not merge anything before this is fixed or reverted, whatever.

@DaanHoogland I don't see any reason for closing master in this case. If an issue is noticed after a merge, it definitely needs to be tracked and fixed. Releasing a branch can definitely be blocked if the issues (particularly regressions) are not addressed but I don't see any reason to close master for other commits. I think that closing master makes sense only if build is broken or there is a blocking regression introduced after the last release.

Guys, LGTMs or not, there were no integration test results posted so it shouldn't have been merged.

Hey Guys, Sorry I have been MIA on this.  I have been traveling and have limited connectivity.
I am going to revert this merge.  There are visible problems with this merge and there were no integration tests run.  @rafaelweingartner has offered to merge these changes into #1444, which fix the issues in this commit and allow us to run integration tests on this commit before merging it in.
Sorry for the delay following up on this and getting a clear resolution in place.

Can we continue the conversation at #1444 please.  This commit has been reverted and has been pulled into #1444 to continue QA of the code.  Please come contribute at #1444 and continue getting this code ready for production.  Thx...


Odd combobox behaviour....

I create a Combo like this:

Combo *filter = combo_create();
layout_combo(filtLayout, filter, 3, 0);
combo_add_elem(filter, "1+ stars", NULL);
combo_add_elem(filter, "2+ stars", NULL);
combo_add_elem(filter, "3+ stars", NULL);
combo_add_elem(filter, "4+ stars ", NULL);
combo_add_elem(filter, "5 stars", NULL);
combo_add_elem(filter, "(no filter)", NULL);
combo_add_elem(filter, "(unrated)", NULL);

That works perfectly, and shows:

But what I really want is this:

combo_add_elem(filter, "★%", NULL);
combo_add_elem(filter, "★★%", NULL);
combo_add_elem(filter, "★★★%", NULL);
combo_add_elem(filter, "★★★★%", NULL);
combo_add_elem(filter, "★★★★★", NULL);
combo_add_elem(filter, "(no filter)", NULL);
combo_add_elem(filter, "(unrated)", NULL);

which does not show correctly, as some elements are discarded:

What could be wrong?

Hi @SamSandq! I add this new case to the ComboBox review.
Related to: https://github.com/frang75/nappgui_src/issues/81 https://github.com/frang75/nappgui_src/issues/68

FYI, the popup control works perfectly, and in my case is also better, since I do not wish to be able to edit the values.
However, I noticed that there is no way to access the text of the selected popup choice, only the index. Perhaps a

char_t *txt = popup_text(myPopup);

or

char_t *txt = popup_text(myPopup, index);

could be considered.

Thanks for update. Therefore, I leave this issue open. Full review of ComboBox is mandatory.
You can get the text of selected Item in OnSelect event as part of EvButton event parameters. But also is a good idea to add const char_t *popup_get_text(const PopUp *popup, const uint32_t index) to public interface.

Yes, I am using that to get the selected text. But what I am after is to
get the text of any item using its index.
Cheers,
Sam
Sam
Den mån 15 jan. 2024 kl 06:19 skrev Francisco García Collado <
@.***>:

Thanks for update. Therefore, I leave this issue open. Full review of
ComboBox is mandatory.
You can get the text of selected Item in OnSelect event as part of
EvButton event parameters. But also is a good idea to add const char_t
*popup_get_text(const PopUp *popup, const uint32_t index) to public
interface.
popup_text.png (view on web)
https://github.com/frang75/nappgui_src/assets/42999199/7f75c8ed-f492-4f90-969c-4fa92bc5cdc3
—
Reply to this email directly, view it on GitHub
https://github.com/frang75/nappgui_src/issues/86#issuecomment-1891277653,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AMAVI5VJXPANYU5HJJPOYSLYOSU3VAVCNFSM6AAAAABBXBANHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJRGI3TONRVGM
.
You are receiving this because you were mentioned.Message ID:
@.***>

Added popup_get_text() https://github.com/frang75/nappgui_src/commit/8177e951cdb5a0d9670e7a499491d209a8dfbc9c.


vstest.executionengine.exe has stopped working

Dear Microsoft Team,
after you sugessted not executing the integration tests on the build server (https://github.com/Microsoft/vsts-agent/issues/883)  I set up a new VM where our application, the tests agent and the tests binaries gets deployed. The deployment works fine but when the tests gets executed the test runner is always crashing after some time (it is mostly running about 70 minutes).
The new VM is also a Windows 2008R2 Server (like before). I attached the screenshot that shows how the test runner crashes and also I attached the DTAExecutionHost.exe.log file and two event viewer log screenshots.
Please help us soon. It is really important for us to run our integration tests every day.
Thanks for your help.
With kind regards
Florian Ertelt
DTAExecutionHost.exe.txt

@k3flo Can you please attach the process dump when it crashes?
Are you able to run these tests using "vstest.console.exe" in the same machine?

@nigurr the dump file is 1 GB big (zipped).
I upladed it at uploadfiles.io: https://ufile.io/c1db21
If I should upload it somewhere else, please let me know.

@k3flo thanks alot

Tracking here: https://github.com/Microsoft/vstest/issues/704


v2 auth-ledger issue with Paho

Im investigeting new (v2) auth-ledger hook, but i believe that there is some issue there.
Looks like golang-paho is not receiving ErrBadUsernameOrPassword packet when auth is failing on broker side.
Simple code to reproduce:
package t

import (
	"log"

	"github.com/mochi-co/mqtt/v2"
	"github.com/mochi-co/mqtt/v2/hooks/auth"
	"github.com/mochi-co/mqtt/v2/listeners"
)

func New() (*mqtt.Server, error) {
	mqttServer := mqtt.New(&mqtt.Options{})

	if err := mqttServer.AddHook(new(auth.Hook), &auth.Options{
		Ledger: &auth.Ledger{
			Auth: auth.AuthRules{
				{Username: "test", Password: "test"},
			},
		},
	}); err != nil {
		return nil, err
	}

	listener := listeners.NewTCP("tcp", ":1883", nil)

	if err := mqttServer.AddListener(listener); err != nil {
		return nil, err
	}

	go func() {
		if err := mqttServer.Serve(); err != nil {
			log.Fatalf("mqttServer.Serve worker died: %s", err)
		}
	}()

	return mqttServer, nil
}

package t

import (
	"errors"
	"testing"

	paho "github.com/eclipse/paho.mqtt.golang"
	"github.com/stretchr/testify/require"
)

func TestNew(t *testing.T) {
	b, err := New()
	require.Nil(t, err)

	mqttClient := paho.NewClient(paho.NewClientOptions().
		AddBroker("<IP_ADDRESS>:1883").
		SetUsername("wrong-username").
		SetPassword("wrong-password").
		SetAutoReconnect(false).
		SetConnectRetry(false))

	token := mqttClient.Connect()
	token.Wait()

	require.Equal(t, errors.New("bad user name or password"), token.Error())

	b.Close()
}

I would expect that paho ConnectToken will return error, but it looks like from paho perspective connection was successfull.
4:28PM INF added hook hook=auth-ledger
4:28PM INF loaded auth rules acl=0 authentication=1 hook=auth-ledger
4:28PM INF attached listener address=:1883 id=tcp protocol=tcp
4:28PM INF mochi mqtt starting version=2.1.1
4:28PM INF mochi mqtt server started
4:28PM INF client failed authentication check hook=auth-ledger remote=<IP_ADDRESS>:60562 username=wrong-username
4:28PM WRN error="bad username or password" listener=tcp
4:28PM INF client failed authentication check hook=auth-ledger remote=<IP_ADDRESS>:60563 username=wrong-username
4:28PM WRN error="bad username or password" listener=tcp
--- FAIL: TestNew (0.01s)
    main_test.go:25:
        	Error Trace:	main_test.go:25
        	Error:      	Not equal:
        	            	expected: *errors.errorString(&errors.errorString{s:"bad user name or password"})
        	            	actual  : <nil>(<nil>)
        	Test:       	TestNew
FAIL
exit status 1
FAIL	t	0.539s

Hi @bkupidura - Thank you for finding and raising this issue!
I looked into this and it's because MQTT v3 has a different specification for the (very few) Connack return codes it provides. In this case the broker was sending the error correctly, but the byte value was incorrect for v3.
I have put together a v5 to v3 code map which seems to fix this issue, and is released in v2.1.4. Give it a try and see if it works as you would expect.

@mochi-co can confirm, new version works as expected! Thanks!


Accessor aliases

In order to reduce the verbosity of programming with SYCL accessors,
this proposal aims to reduce the number of template parameters
to just 2 from the current 5.
It achieves this by slightly revising
how to treat read-only data
and adding alias templates
based on the access target.

Main changes

Default accessor template parameters
Simplify access modes
Alias templates based on access target
Extend the handler class

Accessor alias templates

`constant_buffer_accessor
host_accessor

Treat const T the same as access::mode::read

Simplifies a lot of code

Define implicit conversions for equivalent types
Implicit conversions that add const
Overload for handler::require that also takes an access mode

To weaken the access mode

Default all accessor template parameters

Expect the data type
Default to an accessor to global buffer with read-write access
Assumes accessors can be a placeholder without template parameter

Discussed some considerations and alternatives
Examples of reduced verbosity

That looks like a good simplification.
You could also add some CTAD if compiled with C++17 or higher.

@keryell I've greatly simplified the proposal, and I've added a section on CTAD. I think I would need to add deduction guides to the accessor class, I might still add that. There's also the problem that CTAD doesn't work on alias templates in C++17, so maybe I should add something for C++20 as well?

That looks good. Probably you need to coordinate with Intel to unify the simplifications they are pushing for too.

I've pushed revision 0.3, which focuses a lot more on CTAD and makes host_accessor a separate type


Save genes selected in DGE/correlations tabs for use in plots tab

This is a nice-to-have. This could be as simple as adding checkboxes in the DGE table beside each gene, and then adding a "send genes to plots tab" button that would update the plots tab input with all the checked genes when pressed.
Important notes for development:

Please use raw HTML checkboxes instead of shiny inputs or awesomeCheckboxes! If you use Shiny inputs, performance may be severely hobbled. The edit and delete buttons in the subset interface provide an example of how to create elements using HTML code, and how to get the app to react to those elements using only one observe statement, instead of one statement per row.
A warning should be placed underneath the button saying the genes selected in the plots tab will be erased. Either that, or pressing the button should not erase genes that are already present in the plots tab. If doing the latter, updating the plots tab features input, the selected parameter should be set to include the features already present, if any, plus the ones sent over from the dge tab. It might be better to just erase the genes though, since the buildup of selected genes could get excessive.


Article has broken links to Deployment Manager

This article has a number of links referencing Azure Deployment Manager.  These links are broken and should be fixed or removed.

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

ID: 7d6e8b74-b3e5-8acd-f517-39d5137cffc4
Version Independent ID: e1d6ffde-14d9-8502-2ff2-7a8399de86dc
Content: Deployment Stamps pattern - Cloud Design Patterns
Content Source: docs/patterns/deployment-stamp.md
Service: architecture-center
Sub-service: design-pattern
GitHub Login: @johndowns
Microsoft Alias: pnp

Thanks for reporting - this issue is under review. This is a Microsoft Internal DevOps Bug ID # 526321

I will fix this in the next few days.

Resolved in the private repo.

It appears your feedback has been addressed.  If you are still experiencing issues, please re-open with additional details.


Fixing errors between Ruby port and the original JS

Hey Sebastian,
When overlaying the boxes from the gem vs the original JS algo, I noticed some discrepancies. I fixed the issues and they are now a perfect match. Thanks for the gem!

Thanks for fix!


Data folder structure

Great work! The structure of the Data folder seems to be inconsistent. Huggingface data only has 2 sub-directories, few-shots and test, however, the 'run_partslip++' seems to be  needing img_sp subdirectory as well. Let me know if I am missing something.

Thanks! 'img_sp' is generated by 'gen_sp.py', you can run that script to generate files in this folder.


Reorder project using Drag and Drop

Is your feature request related to a problem? Please describe.
It's not possible to reorder project
Describe the solution you'd like
Possibility of reorder project with drag and drop

Feature is available on production, I let this issue open during few days, waiting for users tests


Do not attempt to make a suggestion if the typed name is just one character

This is just annoying:
q
# => NameError: undefined local variable or method `q' for main:Object
#
#    Did you mean? #!
#                  _

Better to not suggest anything.

It turns out that sometimes it makes sense to do suggest. One character variables could be used while debugging.
@o = 1
o
# => NameError: undefined local variable or method `o' for main:Object
#
#    Did you mean? @o
#                  #!
#                  _


Map is not expanded on timeseries format - [object Object]

Hi,
We have a CH schema with a field defined as Map(String,String). When we try to perform a simple query from Grafana on the TimeSeries panel and as TimeSeries format, the Map value is not expanded and not included as part of the serie. It is working as "Table format" mode.
The idea is to be able to retrieve the value as a dict (or map[string]string) and use the Extract Field Grafana transformation to generate the desired columns, but the generated dataframe seems to concat the string values with an object Object result, being unable to use it.
The following snippet is the "Support" feature that extracts the result Dataframe processed by the backend, with no transformations. There you can check that the generated dataframe is built with Name - Value - Time, with the non-expanded map is set as [object Object] on the dataframes name

Key
Value

Panel
timeseries @ 10.0.3 (eb8dd72637)

Grafana
10.0.3 (eb8dd72637) // Open Source

Panel debug snapshot dashboard
{
  "panels": [
    {
      "datasource": {
        "type": "grafana",
        "uid": "grafana"
      },
      "fieldConfig": {
        "defaults": {
          "custom": {
            "drawStyle": "line",
            "lineInterpolation": "linear",
            "barAlignment": 0,
            "lineWidth": 1,
            "fillOpacity": 0,
            "gradientMode": "none",
            "spanNulls": false,
            "showPoints": "auto",
            "pointSize": 5,
            "stacking": {
              "mode": "none",
              "group": "A"
            },
            "axisPlacement": "auto",
            "axisLabel": "",
            "axisColorMode": "text",
            "scaleDistribution": {
              "type": "linear"
            },
            "axisCenteredZero": false,
            "hideFrom": {
              "tooltip": false,
              "viz": false,
              "legend": false
            },
            "thresholdsStyle": {
              "mode": "off"
            }
          },
          "color": {
            "mode": "palette-classic"
          },
          "mappings": [],
          "thresholds": {
            "mode": "absolute",
            "steps": [
              {
                "color": "green",
                "value": null
              },
              {
                "color": "red",
                "value": 80
              }
            ]
          }
        },
        "overrides": []
      },
      "gridPos": {
        "h": 13,
        "w": 15,
        "x": 0,
        "y": 0
      },
      "id": 2,
      "options": {
        "tooltip": {
          "mode": "single",
          "sort": "none"
        },
        "legend": {
          "showLegend": true,
          "displayMode": "list",
          "placement": "bottom",
          "calcs": []
        }
      },
      "targets": [
        {
          "refId": "A",
          "datasource": {
            "type": "grafana",
            "uid": "grafana"
          },
          "queryType": "snapshot",
          "snapshot": [
            {
              "schema": {
                "name": "id1, [object Object]",
                "fields": [
                  {
                    "name": "Time",
                    "type": "time",
                    "config": {}
                  },
                  {
                    "name": "Value",
                    "type": "number",
                    "config": {}
                  }
                ]
              },
              "data": {
                "values": [
                  [
<PHONE_NUMBER>000,
<PHONE_NUMBER>000
                  ],
                  [
                    1,
                    null
                  ]
                ]
              }
            },
            {
              "schema": {
                "name": "id2, [object Object]",
                "fields": [
                  {
                    "name": "Time",
                    "type": "time",
                    "config": {}
                  },
                  {
                    "name": "Value",
                    "type": "number",
                    "config": {}
                  }
                ]
              },
              "data": {
                "values": [
                  [
<PHONE_NUMBER>000,
<PHONE_NUMBER>000
                  ],
                  [
                    null,
                    1
                  ]
                ]
              }
            },
            {
              "schema": {
                "name": "id3, [object Object]",
                "fields": [
                  {
                    "name": "Time",
                    "type": "time",
                    "config": {}
                  },
                  {
                    "name": "Value",
                    "type": "number",
                    "config": {}
                  }
                ]
              },
              "data": {
                "values": [
                  [
<PHONE_NUMBER>000,
<PHONE_NUMBER>000,
<PHONE_NUMBER>000
                  ],
                  [
                    null,
                    null,
                    1
                  ]
                ]
              }
            }
          ]
        }
      ],
      "title": "Reproduced with embedded data",
      "transformations": [],
      "type": "timeseries"
    },
    {
      "gridPos": {
        "h": 7,
        "w": 9,
        "x": 15,
        "y": 0
      },
      "id": 5,
      "options": {
        "content": "
\n
\n      Panel	\n      timeseries @ 10.0.3 (eb8dd72637)	\n
\n
\n      Queries	\n      A[vertamedia-clickhouse-datasource]	\n
\n
\n      Transform	\n      	\n
\n
Data	 3 frames, 6 fields, 7 rows
\n    \n
\n      Grafana	\n      10.0.3 (eb8dd72637) // Open Source	\n
\n
",
        "mode": "html"
      },
      "title": "Debug info",
      "type": "text"
    },
    {
      "id": 6,
      "title": "Original Panel JSON",
      "type": "text",
      "gridPos": {
        "h": 13,
        "w": 9,
        "x": 15,
        "y": 7
      },
      "options": {
        "content": "{\n  \"datasource\": {\n    \"type\": \"vertamedia-clickhouse-datasource\",\n    \"uid\": \"caf89ca7-12da-4a25-9974-5fc93dc4e57a\"\n  },\n  \"fieldConfig\": {\n    \"defaults\": {\n      \"custom\": {\n        \"drawStyle\": \"line\",\n        \"lineInterpolation\": \"linear\",\n        \"barAlignment\": 0,\n        \"lineWidth\": 1,\n        \"fillOpacity\": 0,\n        \"gradientMode\": \"none\",\n        \"spanNulls\": false,\n        \"showPoints\": \"auto\",\n        \"pointSize\": 5,\n        \"stacking\": {\n          \"mode\": \"none\",\n          \"group\": \"A\"\n        },\n        \"axisPlacement\": \"auto\",\n        \"axisLabel\": \"\",\n        \"axisColorMode\": \"text\",\n        \"scaleDistribution\": {\n          \"type\": \"linear\"\n        },\n        \"axisCenteredZero\": false,\n        \"hideFrom\": {\n          \"tooltip\": false,\n          \"viz\": false,\n          \"legend\": false\n        },\n        \"thresholdsStyle\": {\n          \"mode\": \"off\"\n        }\n      },\n      \"color\": {\n        \"mode\": \"palette-classic\"\n      },\n      \"mappings\": [],\n      \"thresholds\": {\n        \"mode\": \"absolute\",\n        \"steps\": [\n          {\n            \"color\": \"green\",\n            \"value\": null\n          },\n          {\n            \"color\": \"red\",\n            \"value\": 80\n          }\n        ]\n      }\n    },\n    \"overrides\": []\n  },\n  \"gridPos\": {\n    \"h\": 8,\n    \"w\": 12,\n    \"x\": 0,\n    \"y\": 0\n  },\n  \"id\": 1,\n  \"options\": {\n    \"tooltip\": {\n      \"mode\": \"single\",\n      \"sort\": \"none\"\n    },\n    \"legend\": {\n      \"showLegend\": true,\n      \"displayMode\": \"list\",\n      \"placement\": \"bottom\",\n      \"calcs\": []\n    }\n  },\n  \"targets\": [\n    {\n      \"database\": \"test\",\n      \"datasource\": {\n        \"type\": \"vertamedia-clickhouse-datasource\",\n        \"uid\": \"caf89ca7-12da-4a25-9974-5fc93dc4e57a\"\n      },\n      \"dateColDataType\": \"\",\n      \"dateLoading\": false,\n      \"dateTimeColDataType\": \"time\",\n      \"dateTimeType\": \"DATETIME\",\n      \"datetimeLoading\": false,\n      \"extrapolate\": true,\n      \"format\": \"time_series\",\n      \"formattedQuery\": \"SELECT $timeSeries as t, count() FROM $table WHERE $timeFilter GROUP BY t ORDER BY t\",\n      \"intervalFactor\": 1,\n      \"query\": \"SELECT\\n    $timeSeries as t,\\n    count(),\\n    id,\\n    attributes\\nFROM $table\\n\\nWHERE $timeFilter\\n\\nGROUP BY\\n    t,\\n    id,\\n    attributes\\nORDER BY t\\n\",\n      \"rawQuery\": \"SELECT\\n    (intDiv(toUInt32(time), 1) * 1) * 1000 as t,\\n    count(),\\n    id,\\n    attributes\\nFROM test.map_table\\n\\nWHERE time >= toDateTime(1694516084) AND time <= toDateTime(1694517884)\\n\\nGROUP BY\\n    t,\\n    id,\\n    attributes\\nORDER BY t\",\n      \"refId\": \"A\",\n      \"round\": \"0s\",\n      \"skip_comments\": true,\n      \"table\": \"map_table\",\n      \"tableLoading\": false\n    }\n  ],\n  \"title\": \"Panel Title\",\n  \"transformations\": [],\n  \"type\": \"timeseries\"\n}",
        "mode": "code",
        "code": {
          "language": "json",
          "showLineNumbers": true,
          "showMiniMap": true
        }
      }
    },
    {
      "id": 3,
      "title": "Data from panel above",
      "type": "table",
      "datasource": {
        "type": "datasource",
        "uid": "-- Dashboard --"
      },
      "gridPos": {
        "h": 7,
        "w": 15,
        "x": 0,
        "y": 13
      },
      "options": {
        "showTypeIcons": true
      },
      "targets": [
        {
          "datasource": {
            "type": "datasource",
            "uid": "-- Dashboard --"
          },
          "panelId": 2,
          "withTransforms": true,
          "refId": "A"
        }
      ]
    }
  ],
  "schemaVersion": 37,
  "title": "Debug: Panel Title // 2023-09-12 13:26:31",
  "tags": [
    "debug",
    "debug-timeseries"
  ],
  "time": {
    "from": "2023-09-12T10:56:31.582Z",
    "to": "2023-09-12T11:26:31.582Z"
  }
}

Repro case:

Create DB and table with Map(String,String) as column

> CREATE DATABASE test;

> CREATE TABLE test.map_table
(
    `time` DateTime,
    `id` String,
    `attributes` Map(String, String)
)
ENGINE = MergeTree
PARTITION BY toYYYYMM(time)
PRIMARY KEY (toDate(time), id)
ORDER BY (toDate(time), id)
TTL toStartOfMonth(time) + toIntervalMonth(12 * 5)
SETTINGS index_granularity = 8192, ttl_only_drop_parts = 1;

Insert values to mock a TimeSeries:

> INSERT INTO test.map_table values (now(), 'id1', {'key1': 'value1', 'key2':'value2'})
> INSERT INTO test.map_table values (now(), 'id2', {'key1': 'value1', 'key2':'value2'})
> INSERT INTO test.map_table values (now(), 'id2', {'key1': 'value1', 'key2':'value2'})

Create a simple Grafana graph:

SELECT
    $timeSeries as t,
    count(),
    id,
    attributes
FROM $table
WHERE $timeFilter
GROUP BY
    t,
    id,
    attributes
ORDER BY t

Check that the attributes field is being set as an string: object Object and part of the name (as dataframe):

Thanks @Slach for reading it and setting up as 2.6.0 milestone.
I think the current issue, is also related with the current dataframes: the plugin is using and old implementation (Name/Time/Value), which seems a bit abandoned by the Grafana team... (#478 ) and we cannot take profit of the different "labels" features,
Sorry about the following question, but it is an stopper for us and our internal roadmap:
Do you have an estimated release date for 2.6.0?

no we don't have ETA
you could try to use Grafana Labs plugin for ClickHouse https://grafana.com/grafana/plugins/grafana-clickhouse-datasource/

As a quick workaround you could use:
SELECT
    $timeSeries as t,
    count() AS value,
    concat(toString(id),' ', toString(attributes)) AS label
FROM $table
WHERE $timeFilter
GROUP BY
    t,
    label
ORDER BY t

similar with https://github.com/Altinity/clickhouse-grafana/issues/189
maybe we need to make toString on plugin-side on JavaScript and Golang alerts parts


Spread doesn't work in IE 9

Noticed this while running Traceur-compiled compat-table tests in IE 9:

it is not clear to me why IE9 would behave differently here? IE9 does not support strict mode so maybe that could be the problem. I need more information.

Something else:
>> $traceurRuntime.toProperty(Symbol.iterator) in Array.prototype
false

I'm getting the following error in IE9 -
   undefined: 'Float32Array' is undefined in<EMAIL_ADDRESS>

The Float32Array case makes a lot of sense because IE9 does not support that. If you need IE9 I would just disable the Math polyfill. IE9 has a few other issues so I'm not sure I would bother with it.

@arv I was under the impression IE9 was a supported browser for Traceur though?

@guybedford There are definitely cases where IE9 and even IE10 will not have the right semantics for all the features Traceur supports. For example, IE9 has no strict mode (which means unresolved identifiers are not throwing on set) and IE10 has no writable [[Prototype]] (which means no class side inheritance).

it could be worth to handle disabling not supported polyfill(s) in traceur code. in this case traceur doesn't work in ie9 by default and btw it was hard (for me) to find this issue on gooogle.

sorry, one more question. I cant find in docs how to disable math polyfill?

Maybe this answers your question. https://github.com/google/traceur-compiler/wiki/Building-custom-Traceur-runtimes.
We should probably feature detect Float32Array. I know es6-shim has a solution that works without Float32Array but it is little over hundred of lines.
https://github.com/paulmillr/es6-shim/blob/master/es6-shim.js#L365-L486

Filed #1703 for dealing with Float32Array.

its a little hard for my mind :D but as I understand you will patch it with some es6-shime code? thanks

Should be working now.


Can branch gh-892-map-store-impl be deleted?

Issue https://github.com/gchq/Gaffer/issues/892  added support for Hazelcast map implementations. The issue was apparently merged before closing. Can the associated branch be zapped?

Yes it seems like it was superseded by gh-892-map-store-updates which got merged, will delete now.


Clarify what is meant by definition location for textDocument/definition

Currently, the protocol states that:
The goto definition request is sent from the client to the server to resolve the definition location of a symbol at a given text document position.

It isn't clear, in the case of a struct Foo, whether this definition location should encompass just the token Foo, or the entire body of the definition of the Foo struct:
type Foo struct {
    Bar bar
}

It seems like it should resolve to the entire body because:

Visual Studio Code hover preview depends on it spanning the body.
It is more useful for many kinds of analysis e.g. determining function dependencies.

Hi @uforic, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
The agreement was validated by Microsoft and real humans are currently evaluating your PR.
TTYL, MSBOT;

@uforic VS Code hover doesn't depend on the definition request. I am not sure that we should spec this so strictly. Can you explain more why you think this is necessary.

Ah, to be more specific - when you press cmd, or control in the case of Windows, while hovering in VSCode, you execute a definition request, and a preview of the definition appears , as in this video.
The code that renders that preview of the definition, which is here, is a conditional statement - there is a comment that says // if we have a range, take that into consideration for the "to" position, otherwise fallback to MAX_SOURCE_PREVIEW_LINES.
This seems to imply that in the best case scenario, the entire range of the definition would be returned in the textDocument/definition request, so that the preview could just render the definition, and not naively render MAX_SOURCE_PREVIEW_LINES as it does in the fallback scenario.
But as for other reasons, here are some of analysis we've thought about that would be enabled by having the range encompass the entire definition:

Detect if the code inside a method changes (i.e. someone decided to "watch" a definition, we'd like to know if a git diff touches lines inside that method).
A visualization that shows what % of a file is taken up by which definitions
If we issue a textDocument/references command, we'd like to be able to detect which definitions the references are occurring in. If we call textDocument/documentSymbol on the file where a reference occurs, if we had ranges, it'd be trivial to determine which definition the reference occurs in.

Thanks for the feedback - let me know what you think!

There are three separate locations I might want to fetch:

The range of the symbol itself, i.e. just the "Foo" in "type Foo struct {...}"

The range of the declaration construct that defines Foo, i.e. the entire string "type Foo struct {...}"

3 .The range that includes doc-blocks, i.e. the entire string "/// Use this struct at your own risk\r\ntype Foo struct {...}"
Option [2] (which you had in your PR) doesn't seem that useful... if I'm doing a preview-on-hover then I'd like to see [3].
Option [3] has the problem that it's ill-defined for languages that have woolly conventions rather than crisp rules about where a docblock starts and ends. But it'd be the most useful for hover-preview.
Option [1] is the best-defined one of the three. As far as I can tell, it's what's used at the moment by language server implementations for textDocument/definition and also consistent with textDocument/documentSymbols and workspace/symbols.
If I'm right that option [1] is what implementations currently do, then we shouldn't change the protocol under their feet. If anything, either [2] or [3] should be added to the results.

Re: Option 3 - Given that the response from textDocument/hover returns the marked string which includes doc-blocks, is it superfluous to also return those doc strings during textDocument/definition?
Definitely hear you re: the existing implementations point, most of the langservers I've seenreturn just the token because highlighting that token after jumping to definition is the expected behavior. Seem like adding or modifying would be the way to go.

The major idea of the protocol is to get a decent UI integration not to have a correct language implementation. This is why in VS Code we use some fallback rules if the range is not provided or looks odd.
If we start to put a firm definition in and users will use the language server for program analysis (for which it is not designed right now) and then they will file issues servers not honoring the spec although the tools UI still does reasonable enough things with the result.
So in general I would like to keep the spec in the sense that the result is reasonable enough for a UI presented to a user which is able to decode the information in a meaningful way. I agree that this has to go in the spec in general.
If we want to have a program analysis LSP then I thing we should have a different set of requests servers can opt in to provide.
Open for opinions.

I would suggest that textDocument/definition only include the token that represents the name (i.e., option 1).  While for a language like Go, an entire struct declaration may be relatively brief (a few lines, maybe up to a couple of dozen if there's a lot of members and comments), other languages are not so terse.  Consider C#, where the equivalent of a struct may span almost the entirety of a hundred- or  thousand-line file (i.e., here).  If a C# class definition is broken into several files (using partial classes), then the problem is conflated.
I can understand wanting to be able to get ranges where there are relevant implementations related to a token (i.e., where are its functions defined, etc), but I think that should be a separate, perhaps optional, request.

Agree with @object88 statement. The basic info / range should be the actual token that has the name of the symbol. But... I can also see that some UI bits may work better if they also have the whole range of the definition (e.g. for a function like 'peek definition' it would be nice if the tool can only show the actual definition and not just some arbitrary amount of text around the name).
So I think it would be useful to allow server implementation to include the 'whole range' as an optional attribute if it makes sense.

What about simply introducing a caretPosition into the Location type?

@mickaelistria could you be more specific? What should LSP servers send as this caretPosition, and how should clients deal with it?

LSP servers should send the recommended position for the caret when returning this Location. For example, when returning a whole AST node, the caretPosition would for example return the position just before the name of the node in the declaration.
So clients would be using the range as they want (for example to show the node it in some hover, or to provide some operation such as "copy target node", "delete target node"...), but would strongly be recommended to place the caret where suggested when doing a "go to".

I don't like the 'caret' suggestion. We are discussing about confusion about the meaning of the 'range'  ( @ljw1004 sumarized nicely what three reasonable interpretations are).
Introducing the extra 'caretPosition' does nothing to address that confusion.
To make this more explicit, our language server has used Interpretation 1 and returns only the range of the name token of the symbol (i.e 'Foo' in Lucius example). It wouldn't help much to supply an extra caret position with that. And you shouldn't be proposing operations like 'delete definition' which would only delete the name.

@mickaelistria I see. So let's spell out your proposal more precisely:

The three LSP requests that return a Location (GoToDefinition, FindReferences, DocumentSymbols) all do so because they're providing information about where a symbol is used or defined.

The Location type contains Location.uri and Location.range. Current LSP servers, when asked to give the Location of a use-site, invariably give the range of the use of the symbol identifier itself. Current LSP servers, when asked to give the Location of a definition-site, are mixed: most give the range just of the symbol identifier itself, but some give the range of a the whole declaration (which is a vague but practically useful concept).

You propose that the Location type contain a new optional element, Location.identifierStartPosition.

You propose that when Location is used to return a use-site, then identifierStartPosition will be the start of the symbol. You propose that when it's being used to return a declaration-site, then LSP servers will change so that range is always the full range of the declaration, and identifierStartPosition is the start position of the identifier.

You propose that for clients which wish to highlight the whole identifier, they should look at the start position, and then use their own custom language-specific regex (it's not part of LSP) to figure out how long that identifier is.

EVALUATION
It's not good to provide only the identifierStartPosition. It is important to provide the identifierRange. That's because there's no good robust way of determining the length of an identifier.
It's not good that Location.range should mean two such different things when used for a call-site vs a use-site. It would be better for Location.range to always refer universally to the range of the symbol in both cases. And then we can add a new optional field Location.declarationRange, used only for declaration-sites, which provides the full range of the declaration. (Again a vague concept and it's up to the language to decide what is the best range. I suggest that typically it shouldn't include preceding doc-blocks).

I understand that's what's being discussed is the meaning of range and I believe that the range should be the whole range of the AST node, and not only the identifier range. I simply don't get what value the users can get by having the range of the symbol selected in a declaration or a definition.
If a user does Ctrl+Click on a text in Eclipse IDE to move to declaration, just placing the caret where the symbol is defined as good (and even better) than selecting the name of the class for instance. Simply because when user does a Ctrl+Click on a text, the next action is extremely unlikely to require selection of the name of the symbol; users are more likely to scroll on the page, use Find References, some Rename or whatever. None of those make selecting th symbol name useful. Ie interpretation 1 described above is not more interesting than the other ones, and can be replaced by a caretPosition or similar without losing value in the protocol.
Since selecting symbol name seems useless to me, I believe the range is much more valuable being the whole node range as it enables more powerful feature to be implemented; and as a client of the LSP and user of IDEs, what I'm interested in "go to defintion" is only to place the caret at the right position.
IIRC, LSP4E deliberately ignores the range and only uses the 1st position to simply place the caret, because we found it better in practice.
So overall, I'm for range to be the whole node, and for an additional optional value to specify a caret position, because I believe it's what maps better user actions and it enables more powerful workflows.

I'm not sure how selecting an entire struct or class block would be more helpful when performing Find References or Rename, over selecting just the name token.  In the case of a language like C# of Java, where everything related to a class is defined within the class block, that seems to be very noisy.  Can you help me understand?

I'm not sure how selecting an entire struct or class block would be more helpful when performing Find References or Rename, over selecting just the name token.

I'm asking the same question, but the other way round ;)
Range as entire node is more helpful IMO because it is more correct: the range of an element isn't the name of the symbol, it's the whole node.
I've seen the documentSymbols range attribute interpreted in 2 different ways: whole node and identifier only. The first one provides ability to build a Tree-based view of the object, to navigate in 2 directions between text and tree, while the 2nd one enables specific. Since then, I'm really convinced that having ranges thinking about nodes in general allows to get closer from the AST world which brings a lot of cool possibilities.
For example, in the case of Find References, let's assume you see the search results as a list. If the list contains the identifier only, then deleting an item of the list from the list (which would cascade to deletion of the identifier) drives to a syntax error; if the range represents the whole node, then deleting the search result from this list would delete the whole class/struct, which is a proper AST operation.
For textDocument/definition, I don't have a clear use-case in mind, except the one of "embedding" the definition in the hover to prevent user from switching to another editor to see the content (like Eclipse JDT allows). But this use-case is already pretty interesting. Anyway, my main argument would be that as ranges as whole node for documentSymbols are clearly more powerful and as I like consistency, I would be for ranges expressing the whole AST nodes whenever possible as I trust it would enable user stories plain identifier don't enable.
I think in general, dealing with ranges as close as possible from AST node enables more coarse-grain actions and alternative views on the documents.

that seems to be very noisy

That's more up to the tool to present it in a less noisy way. In Eclipse IDE, when a search result (Find Reference output is map as search results) is multiple lines, it simply shows the 1st few words, so it's not that noisier than showing only the work.

@mickaelistria I agree that the range of the whole declaration has benefits. But I'm struggling to see how to generalize or formalize your idea. Here's a concrete example of ASTs:
Alpha.Beta.gamma(1);

[0]Call
+-[1]QualifiedIdentifier
| +-[2]QualifiedIdentifier
|   +-[3]identifier "Alpha"
|   +-[4]qualifier "Beta"
| +-[5]qualifier "gamma"
+-[6]argument "1"

class Beta {
  static Func<int> gamma
}

[7]ClassDecl
+-[8]identifier Beta
+-[9]member "static..."

In the first case, I did "Find all references" to find all references to symbol Beta. This might give the range for just node [4] in the AST, or it might walk up the tree to find an enclosing node [2] or [1] and give the range of that.
In the second case, I did "Go-to-definition" to find the definition of symbol Beta. This might give the range for just node [7], but you prefer it to walk up the tree to find the enclosing node [9] and give the range of that.
In general, do you propose that locations of use-sites should provide the range of the AST node for the identifier itself, but locations of declaration-sites should walk up the AST to find the smallest declaration node that contains the identifier?

I don't think we should be arguing about whether it should be 'just the name' or 'the whole definition'. I'd rather argue that you probably want to allow the server to decide whether it wants to return one, the other, or both. And its up to the client to make the best use of whatever information, the server chosen to return.
If you only have the 'name range' then you can not really determine what range of text to show in the 'peek' window, so that's not great. But if you only have the 'whole range'  you can not easily determine where the name is, or perhaps, even what the name is. So I think that's not so great either.
Also I don't think we should be arguing to bake a 'single chosen meaning' into the spec. It was, so it seems, deliberately left vague. And so there are servers out there following different interpretations. You don't want to declare some percentage of those servers 'wrong' now, but rather give them a way forward with an augmented spec that tolerates their old use and allows them to become more precise about the intended meaning of what it is they are returning (and augment with additional info).
The caret position not having a good way to determine the 'end' of the name is a non-starter to me. You could just as well make that a 'name range' and if you really don't want to know where it ends, then you can choose to ignore that info. But if you only have the start, then there's no reliable way to determine the end.

Since the LSP is made to allow UI integration and not program analysis we should not try to spec this the same for GoToDefinition, FindReferences and DocumentSymbols. The result should allow for the best user experience and not for the program analysis correctness. I agree that we shouldn't spec a 'single chosen meaning'. We should rather spec what the tool usually does with the result. Something like this:

GotoDefinition: the returned range is used to place the cursor at the starting point of the range when the user navigates to the definition.
DocumentSymbols: the returned range of the location is used to highlight the symbol in the document when the user browses through the list of symbols.

FindReferences: the return range is selected when the user picks a reference from the list.

If we need a different result for other user gestures then I think we should add additional request. The fact that VS Code uses goto definition for a peek preview is not correct and we should better have a different request (e.g. textDocument/peekPreview) to return a result that contains a meaningful range to be presented in a peek preview.

@dbaeumer The DocumentSymbols API is currently used for [1] OutlineView (which would prefer to have entire range from start of declaration to end of }, so it can accurately reconstruct the tree).
Some clients might also use it just to [2] show a list of symbols and highlight whichever symbol the caret is currently inside, so it would prefer to have the whole range too.
It's only the use-case of a popup dialog [3] where you type in a name and scroll through the offerings (as you showed) which benefits from the range being only the symbol.
I think that to further the goal of LSP being a cross-editor API (rather than being narrowly tied to the UI of VSCode) it's important to address the [1/2]. They are after all different ways of displaying the list of available document symbols. I also think it's a non-starter to stick in a new LSP API for every single possible UI in every single editor. Here we have an API that's mostly perfect for [1/2] already, save for a small bit of clarification or an extra field.

@ljw1004 I only took VS Code as an example and the LSP shouldn't be tied to it. What I was trying to say is that GoToDefinition, FindReferences and DocumentSymbols use a Location underneath and that we can't specify what a Location is so we need to specify how the location is interpreted and therefore should look like in the above requests.
But the tools should still meaningful behave if the result is not sticking to it. So I would rather add a outline request that has the concept of a clear hierarchy than enforcing that the result a document symbol request can be used to reconstruct a tree.

So I would rather add a outline request that has the concept of a clear hierarchy than enforcing that the result a document symbol request can be used to reconstruct a tree.

I find this risky for LSP maintainability. If all unspecified behavior drive to a new operation, the number of operations will increase drastically, duplication of concerns is very likely to occur. It would IMHO be much more "proper" to work on the semantics of the existing data-types and operations, even if the only way forward is to deprecate some fields (such as range) in order to introduce disambiguized ones (nodeRange, identifierRange...).
I think LSP could do more with less operations if there was a path towards clarification or deprecation/replacement of ambiguous values. And doing more with less usually scales better in term of maintenance and adoption effort ;)

@mickaelistria I agree with this but it is not the right thing to do to spec a document symbol request in a way that clients can re-infer the hierarchy. If we need a hierarchical outline we should have a request for this.
The idea of the LSP is to talk UI abstractions not AST / compiler abstractions. If we start to add something like a node range and an identifier range we start to change this and IMO this make things more complicated considering that we want to be language agnostic. I still believe that abstraction over the UI across tools is easier than abstracting over AST / compiler concepts across programming languages.
I am all in for making the spec more clear. But I am still very confident that the clarification should happen in terms of UI abstractions not in terms of AST / compile abstractions.


When to use async components?

I noticed in the example code that some of it is async and some of it isn't; what's the logic on what's async and the restrictions on using async for rscx?

short answer:
all the components in the end are async (you can omit the async keyword when writing the components function, but the macro will add it for you, maybe not the cleanest developer experience)
for the long answer:
this was needed since in the html!{} macro I have to use await at the call site and I had no information whether the components were async or not
an idea I had but never implemented was to add a suffix to function names such as you_fn_name__async so that I could discriminate at the call site, but I never needed for this


Do not implicitly scroll a PageView when showOnScreen is called

Fixes https://github.com/flutter/flutter/issues/19523


Improvements for UNIX A.out Loader

As mentioned in #1, another set of improvements for this loader. I've cleaned up, improved and modeled the loader against the ELF loader as a reference.
Among other things:

The a.out file structures are loaded into memory (as overlays if needed) ;
The a.out file structures are marked up in the listing view (header, relocation tables, symbol table, string table) ;
The relocation table is filled out ;
Undefined symbols are allocated from an EXTERNAL memory block (just like for ELF).

It's not quite there yet, I have a couple of relocations/symbols bugs with the Slackware 2.3 C standard library I haven't nailed down and there's some cleaning up left to do. Therefore, I'm marking this as a draft for the time being.
One additional area of improvement would be adding stabs support, but that's probably best done inside a dedicated analyzer (like how DWARF support is implemented inside Ghidra) and out of scope of this loader. I haven't decided yet if I'll write one, that'll depend on where my own reverse-engineering/investigation efforts will lead me.
Like in my first PR, I'm fine with squashing.

I think it's in a good enough state now. There's still some weird stuff going on with some symbols whose addresses are beyond the end of their section, but I ran out of patience wrestling with that ancient junk, so meh.
I've deactivated the ability to load a.out binaries inside a program (supportsLoadIntoProgram()) because the refactoring broke that. I only care about static executables and relocatable objects (as a source for the version tracking tool) for now, so that's good enough for me. I don't know much about a.out shared libraries ; I can probably prepend section names with their file names when using Add To Program... if you need that (that might be enough for Linux a.out shared libraries when overriding the base load address), but I won't implement proper a.out shared library support in this loader myself anytime soon.

Thanks! Since this is my first significant Ghidra contribution, I'm happy to defer to anyone with more knowledge about Ghidra coding style and architecture. It's nice to have it align better with the organization of the ELF loader.
If you could look into re-enabling supportsLoadIntoProgram(), that would be extremely helpful. My primary use case for A.out loading actually requires this. I'll merge this PR in the meantime.

Thanks! Since this is my first significant Ghidra contribution, I'm happy to defer to anyone with more knowledge about Ghidra coding style and architecture. It's nice to have it align better with the organization of the ELF loader.

I'm not that knowledgeable about Ghidra's internals, I know just enough to get into plenty of trouble (https://github.com/boricj/ghidra-delinker-extension). Hopefully these were steps in the right direction for upstreaming your a.out loader.

If you could look into re-enabling supportsLoadIntoProgram(), that would be extremely helpful. My primary use case for A.out loading actually requires this. I'll merge this PR in the meantime.

Do you have documentation on a.out shared libraries somewhere? I can't seem to find anything useful online besides "it sucked hard in ye olden days so we switched to ELF decades ago". I've looked at Slackware 2.3's source code archives of glibc, ldso and Linux kernel and found some answers, but it's hard to piece together a coherent picture from that. What about other operating systems? I've read that the BSDs did it differently, but that's about it...
Also, what level of functionality are you looking for? If it's just loading Linux-style shared libraries by hand I can probably make that work, anything beyond that I probably can't help with.

Also, what level of functionality are you looking for? If it's just loading Linux-style shared libraries by hand I can probably make that work, anything beyond that I probably can't help with.

I don't exactly need support for a.out shared libraries, specifically -- just adding object files to the program being analyzed. My workflow looks like this:

Import a VxWorks (monolith?) image using the generic binary file importer. Manually partition it into .text and .data with the VxWorks fixed/global address space.
Run the VxWorksSymTab_Finder script to locate the named symbol table and use its contents to identify symbols throughout the image.
Import a separate a.out object code file using Add To Program.... In my case, these files are identified in their headers as OMAGIC (0x107). During this step, the code in my original version of the a.out loader would find the symbols previously identified by VxWorksSymTab_Finder, and fix their addresses to point to the true location of each symbol. (The actual fixing was being done by UnixAoutLoader::fixAddress().)

I found that fixing each address to point to the original symbol was very helpful, because then any change to a function signature (for example) would be automatically reflected in calls to that function from the code in the object file.
Would it be possible to restore this functionality? Please let me know if I'm thinking about this the wrong way.

To be honest, after giving it some thoughts I'm not even sure there is a right way to think about this.
Usually, Ghidra handles undefined symbols by defining an EXTERNAL block that holds placeholders and applied relocations against these, so that analysis can proceed as if everything's OK. Each dynamic library, if any, is loaded as a separate program so that they aren't duplicated into multiple programs that uses them.
As far as I can tell, loading libraries into a program isn't the usual workflow. In-tree loaders that do support loading into programs mostly either add raw bits or add debugging symbols ; none of them appear to support loading libraries. However, some recent changes in the relocation table (the Status enum inside Relocation.java in particular) suggest that might change in the future, but I don't know to what extent.
Now, Linux-style a.out shared libraries already break that model because external symbols aren't undefined, every shared object was globally pre-allocated a virtual memory range and the relocations were pre-applied in advance. That means there are no symbols to put inside an EXTERNAL block or even symbols to mark as external ; these references will point to unallocated memory from Ghidra's point of view unless that shared library is loaded, but that would duplicate these libraries into each program that uses them...
But there's also the problem that Ghidra really doesn't expect the relocation table to be touched after the initial load (my attempts to change that for unrelated reasons didn't reach consensus: https://github.com/NationalSecurityAgency/ghidra/pull/5010). That works out for Linux-style a.out dynamic linking because there are no relocations to process, but that's not the case for other operating systems. In theory, these shared libraries should be loaded alongside the initial program load so as not to break Ghidra's assumptions about its relocation table ; in practice that would be a mess with a.out because there are no standards for dynamic linking. Even worse, unlike ELF there's no metadata to tell what dynamic structures are where (the list of dynamic libraries to load on Linux being one example that I'm aware of) ; which leads to hacky heuristics like VxWorksSymTab_Finder.java to sort this mess out.
TL;DR I don't know. I'm not knowledgeable enough about Ghidra or a.out to figure it out on my own. I'll look into making loading a.out Linux-style shared libraries work since that at least doesn't break assumptions that Ghidra currently makes and it should be fairly straightforward to do. For the rest, it might be a good idea to open a discussion on Ghidra's side about this. Maybe I've missed something obvious, but I'm too confused right now to make sense of this.

Thank you for your thoughts on this. My workflow is probably a little unusual in that the VxWorks image I'm loading contains code that would normally be shared library code (e.g. functions from libc or its equivalent). The main executive loop then dynamically loads a separate a.out OMAGIC file and passes control to a known function contained therein. The code in the OMAGIC file then references library symbols back in the original image.
As you suggest, it's probably worth a discussion thread back in the Ghidra project.


attempt to update background.js

This pull request isn't passing because the test website was crashing


Circle Configuration

[ ] view / accept / deny member join requests [IF PRIVATE CIRCLE]
[x] member groups and privacy settings
[ ] member management [edit groups, remove from circle, invite members]
[ ] Circle public / private [Private circles requires join requests]
[ ] Private circle questionnaire [motivation for joining and accepting terms]

Invite users for now is only a link to join. [sign in / sign up & accept invite] -> redirect to circle


Pass in type for id columns

Lets us type the resource id correctly (instead of casting it to a string).
Makes the sql more efficient and able to use indexes.

Would it be possible to build an internal map of resource_type -> pk_type during SQLAlchemyOso initialization (same place we register_models()) instead of (A) asking users to manually supply the resource_id_column_type and (B) forcing all resource ID columns to be the same type?

Yeah it's possible. I'm up for getting the type in a more inferred way but it would have to be during the init which happens before register_models so I don't actually have the models at that point.
As far as not forcing all resource IDs to have the same type; Doing that would complicate the sql a lot and the way we store role assignments. Making them all the same type seems like something most people would be ok with and allows us to keep the model as it is.

As far as not forcing all resource IDs to have the same type; Doing that would complicate the sql a lot and the way we store role assignments. Making them all the same type seems like something most people would be ok with and allows us to keep the model as it is.

We basically decided not to do this (https://www.notion.so/osohq/Table-per-role-resource-so-we-can-use-actual-id-columns-instead-of-strings-and-update-all-queries-to-dc8495cca8ba4c5da5a1a520ef5eb46a) so that we could spend less time on this.
@saolsen Did this change make it so that the single item queries are able to use index scans?

@saolsen Did this change make it so that the single item queries are able to use index scans?

Not verified, though we also didn't verify that they couldn't before.

Yeah it's possible. I'm up for getting the type in a more inferred way but it would have to be during the init which happens before register_models so I don't actually have the models at that point.

doesn't register_models happen during SQLAlchemyOso init?

Yeah it's possible. I'm up for getting the type in a more inferred way but it would have to be during the init which happens before register_models so I don't actually have the models at that point.

doesn't register_models happen during SQLAlchemyOso init?

Ah yeah SQLAlchemyOso init. Cool. I think I can manage to get this working.

Not verified, though we also didn't verify that they couldn't before.

The reason I'm asking is after we do this we shouldn't have any queries that have to do table scans, right?
I think the cast in the join that you got rid have would have caused a table scan, but I'm not sure if there's anything else.


Support for PyTorch 2.0

PyTorch 2.0 was released two weeks ago: https://github.com/pytorch/pytorch/releases
To make COMET compatible with installations that updated to PyTorch 2.0 already, we can loosen the version restriction on PyTorch. I ran the unit tests and they all passed. Most changes in 2.0 are backwards compatible, so I wouldn't expect any issues.
closes #119

Sorry. This is not forgotten but I did not had time to look into it.
I'll set aside some time for tomorrow to merge this and bump the pypi version to (2.0.1).


Darzalib error, link expired

could you update the darzalib download?

I decided to private the repository; it will not be available.

Oh, thank you for responding, is there any way to get the cheat tho, or
could you perhaps help me with an error that i get?
~,,
Traceback (most recent call last):
File "C:\Users\ssusi\Desktop\Midnight-main\Proxy.py", line 67, in
main()
File "C:\Users\ssusi\Desktop\Midnight-main\Proxy.py", line 49, in main
with open("data/ProjectileDictionary.pkl", "rb") as f: projDict =
pickle.load(f)
FileNotFoundError: [Errno 2] No such file or directory:
'data/ProjectileDictionary.pkl'
"
Please, reach out via discord,( Dreamas#2975 ) I'd like to talk a little
more
Have a nice day, Slushiis
On Wed, Jul 20, 2022, 15:37 swrlly! @.***> wrote:

I decided to private the repository; it will not be available.
—
Reply to this email directly, view it on GitHub
https://github.com/swrlly/Midnight/issues/3#issuecomment-1190233554, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/ALDCXMVT7VLM75QW6YGZMNLVU7XHDANCNFSM54DFJIJA
.
You are receiving this because you authored the thread.Message ID:
@.***>


Add printer declarations to interface files

Is there any way to combine create_auto_printer_mapper and create_auto_printer_sig_mapper?

Hmmmmm this doesn't build on 4.02.3

@let-def are you by chance available to help with ocaml-migrate-parsetree?

@tekknolagi Yes sure. Sorry for delay.

@let-def not a problem! I think I caught you at near midnight your time. But the problem I'm having is a little different -- I think there are some issues with the difference in arg_label between 4.02 and 4.03 and 4.04.

Ok, I see. You cannot use ppx_tools directly, because it depends on the concrete compiler version which ocaml-migrate-parsetree tries to abstract.

I started ppx_tools_versioned to solve that problem.
So a solution would be to:

use ppx_tools_versioned & ppx_tools_versioned.metaquot_404
add "open Ppx_tools_404" at the beginning of files relying on ppx_tools

(404 as we target 4.04 parsetree :))

Aha! This sounds like a good solution. Is your package available on opam?

I am afraid not yet, I am in the process of writing it, it is not yet a stabe solution but that should happen soon.
In the meantime, opam pin add ppx_tools_versioned https://github.com/let-def/ppx_tools_versioned.git should do it.

@let-def looks like Travis (macOS) passes, but Circle (Linux) fails. Something about not being able to find ocaml-migrate-parsetree for 4.02.3. Thoughts?

You know what, looks like @yunxing is right.  My bad.

@yunxing before this gets merged I want to edit the commit message(s) and squash.

Diff looks good. I will wait for you to refactor the test fix out into a separate PR.

Looks good. Let's merge this!

Just did a rebase on top of master to fix conflicts.

Sorry that my comments will be a little harsh but this is my last rant on pulling deps,
so bear with me : )
<MyRant >
IMO: we need  quality control instead of keep piling dependencies, something is cool does not mean it should be used, if you have one part which is bleeding edge, it is fine, if you have two parts which may be problematic, if you have three, then for sure everything will get shaky.
Here for a very experimental feature(deriving printer), tons of dependencies are pulled in, even if we introduce deriving printer, it is okay to write some boilerplate code instead of using fancy things like ppx_metaquot or ppx_deriving.
Reason is  foundational tech for reason users, it has to be rock solid, we should not introduce "left-pad" kind dependencies, it is fine to write some duplicated code, quality control is very very important.
</>

@bobzhang I agree with the sentiment. Pulling in more dependencies makes things
harder. But here's where I disagree:
I would not necessarily describe adding the deriving printers as merely
"something that is cool". We have a lot of JS converts using Reason at Facebook
and they are very much used to console.loging whatever they want and having
something useful print to the screen. While I can't (easily) provide that kind
of functionality, I made a compromise: make a relatively easy-to-use printer
function for each type they define.
If I were to write that ground-up from scratch, we would indeed pull in fewer
dependencies. That much is clear. But the project would also inherit 1.5-odd
months of intern project to grow and maintain. I'm not saying I write bad code,
but I am certainly not a well-established full-time employee who has the time
and bandwidth to maintain my feature.
Adding a dependency on ppx_deriving (which uses ppx_tools' metaquot ppx and a
function or two from ppx_tools' ast_convenience) shifts the maintainership
responsibility to whitequark and Alain Frisch, long-time OCaml contributors --
and certainly much better programmers than myself.
I would also not describe ppx_deriving and ppx_tools as "a ton of
dependencies". The actual amount of code is small and well-revised. Before
using them, I personally reviewed a not-insignificant portion of the two
projects.
Last, reducing ppx_deriving and ppx_tools to "left-pad" dependencies is a gross
exaggeration. They provide useful APIs and more power than the several-line
"left-pad" dependency.
Again: I agree with the sentiment. But I think that pushing for fewer
dependencies only is the wrong direction to take here.

In general I think it's very tempting to rewrite everything from scratch for better quality and better control over the code. I had several cases in my previous team where we had to rebuild everything in house : From a strong consistent KV store to chassises that can reduce the vibration of hard disks.
At beginning, it felt very cool to rebuild so much interesting stuff from ground up. But we soon paid our price: The person who owns the knowledge becomes a single point of failure. With the high turnover rate in SV. After the owner of a component left, we got a codebase where no one has the knowledge or time maintain. In addition, we found the home made components are not necessarily better than ones that are already available in the industry that were made by better experts than us.
I'm for avoiding dependencies as much as we can. But I want us to ask ourselves some questions before deciding to rewrite a component from scratch: 1) Have we done any due diligence to look for any existing softwares written by people who are better than us? 2) If existing software doesn't fit into our use case, why is that? 3) Can we pay minimum effort to change them to fit into our story?
In our case, I'm happy that Max did the due-diligence for us and found an existing component to reuse. The time and energy that took to reuse it turns out to be much smaller than writing a printer deriver by ourselves (Looking at ppx_deriving_show, I can imagine how many edge cases we will have to fix in the future if we choose implement this logic by ourselves). To be honest, we are also paying a cost to it: We indirectly broke some use cases like Reason-tools (which turned out to be an issue with Rebel, a build system we implemented by ourselves). But I'd say the time we spent to fix those issues is much smaller comparing to the case if we were to build a printer deriver from scratch and to maintain it.
I can totally see where you are coming from and I agree we need better quality control over dependencies to make sure it doesn't break anything. On the other hand, I still think building a entirely home-made component for a experimental feature is too risky given the limited resource we have. I'd suggest let's try this feature and learn the limits of ppx_deriving_show. Then we can decide if we are going to rewrite it or not when it comes out of experimental stage. How does that sound?


vcf2zarr progress bars aren't updated on Mac

For example:
vcf2zarr convert sample.vcf.gz sample.zarr -p 4
    Scan:   0%|                                       | 0.00/1.00 [00:00<?, ?files/s]
 Explode:   0%|                                        | 0.00/9.00 [00:00<?, ?vars/s]
  Encode:   0%|                                            | 0.00/927 [00:00<?, ?B/s]
Finalise: 100%|███████████████████████████████| 21.0/21.0 [00:00<00:00, 1.64karray/s]

Running on a bigger VCF is the same: the first three bars are never updated.
This is using Python 3.10 on a Mac M1 (13.5.2) running under Rosetta.

Great - this is a dupe of #175 then

Seem likely the problem is to do with the shared memory approach taken here. Any insights into what's going on in Mac-land here would be much appreciated!

Mac uses spawn by default, while Linux uses fork - see https://docs.python.org/3/library/multiprocessing.html#multiprocessing-start-methods.
When I change to use fork, it works:
diff --git a/bio2zarr/core.py b/bio2zarr/core.py
index bd31656..730d85f 100644
--- a/bio2zarr/core.py
+++ b/bio2zarr/core.py
@@ -166,6 +166,7 @@ class ParallelWorkManager(contextlib.AbstractContextManager):
         else:
             self.executor = cf.ProcessPoolExecutor(
                 max_workers=worker_processes,
+                mp_context=multiprocessing.get_context("fork")
             )
         self.futures = []

However, it's not clear how safe this is, see https://stackoverflow.com/a/70552892:

fork might be OK as long as you don't use threads

Another suggestion is to use https://docs.python.org/3/library/multiprocessing.html#multiprocessing.Manager.
Finally, in Python 3.14 the default will change away from fork for all platforms.

Another way to do this would be to update progress as results are returned from results_as_completed. This would allow any of the various ways to start a process (fork, spawn etc).
Would that work?

That was super helpful, thanks @tomwhite. I ended up using the shared memory multiprocessing.Value after trying out a lot of different options, and it seemed to the best balance of robustness and responsiveness. I agree the architecture isn't great, coupling over a global variable, but doing progress well is hard.
I switched everything over to "spawn", and got it working again, so should be the same on all platforms now.

Would you mind trying out the latest code to see if the progress works OK please @tomwhite?

I just tried it and it works great! Thanks for fixing it @jeromekelleher
BTW I do get a UserWarning: resource_tracker: There appear to be 3 leaked semaphore objects to clean up at shutdown but the progress is working now.

BTW I do get a UserWarning: resource_tracker: There appear to be 3 leaked semaphore objects to clean up at shutdown but the progress is working now

Good to know - can you open an issue to track this one please?

Any other usability type-things you see on mac would be great to know about btw.


[Maintenance] Replace nalgebra with glam

Background
Nalgebra, I love you, but you're bringing me down.
nalgebra is a great maths library. It does lots of cool stuff.
 So many things that are otherwise simple in other Maths libraries are just mind-bendingly complicated in nalgebra. Its errors are indecipherable, its documentation is a maze of darkness and it uses so many traits that it gets specifically called out in the cargo docs for its heavy use of generics.
So, let's hurl it into the sun and replace it with glam.
TODO

[ ] How do we keep rapier and still have nalgebra?
[ ] Do it

I beg you not to do this

At least use https://docs.rs/nalgebra-glm/latest/nalgebra_glm/ instead