Add 'infer_empty' as an option for extract_athena_types()

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
When infering types from DataFrames if the column is fully empty the infer fails with an error. While I believe this to be the right behaviour in almost all cases I think it would be useful if we could force the empty columns to infer as a type that we pass in.
E.g. Current Situation:
import pandas as pd
import awswrangler as wr

df = pd.DataFrame([[1, 'Foo', None], [4, 'Bar', None], [7, 'Cat', None]],
                  index=['A', 'B', 'B'],
                  columns=['numeric', 'string', 'empty'])

df = wr.catalog.sanitize_dataframe_columns_names(df)

df_types = wr.catalog.extract_athena_types(df)
print(df_types[0])

Results:
UndetectedType: We can not infer the data type from an entire null object column
Describe the solution you'd like
A clear and concise description of what you want to happen.
Ability to force the empty types (for all columns that are empty)
df_types = wr.catalog.extract_athena_types(df, infer_empty=str)

Result:
{'numeric': 'bigint', 'string': 'string', 'empty': 'string'}
Current Workaround:
import pandas as pd
import pyarrow as pa
import awswrangler as wr
from awswrangler._data_types import pyarrow_types_from_pandas, pyarrow2athena

df = pd.DataFrame([[1, 'Foo', None], [4, 'Bar', None], [7, 'Cat', None]],
                  index=['A', 'B', 'B'],
                  columns=['numeric', 'string', 'empty'])

df = wr.catalog.sanitize_dataframe_columns_names(df)

x = pyarrow_types_from_pandas(df, False)

bypass_empty = {}
for k, v in x.items():
    if v == pa.null():
        vtype = 'string'
    else:
        vtype = pyarrow2athena(v)
    bypass_empty[k] = vtype

print(bypass_empty)

P.S. Don't attach files. Please, prefer add code snippets directly in the message body.

Didnt realise I could do this. Thank you for the reply.


Fio server_vm image updates

Description
Adjusting server_vm config so that any VM containerDisk image can be used since the workload is run from chroot into a fio container image (i.e. no custom VM images need to be maintained).
Also adding a fallback VM option if no PVC/HostDisk is specified, use an emptyDisk (ephemeral storage).
Adding nodeSelector support for VMs.
Fixes
#527

Can one of the admins verify this patch?

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@eb8be37). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #685   +/-   ##
=========================================
  Coverage          ?   53.48%
=========================================
  Files             ?        8
  Lines             ?      316
  Branches          ?        0
=========================================
  Hits              ?      169
  Misses            ?      147
  Partials          ?        0

Flag
Coverage Δ

gha
53.48% <0.00%> (?)

python-3.9
53.48% <0.00%> (?)

unit
53.48% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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 eb8be37...9332d97. Read the comment docs.

Thank you @jeniferh !  I'm still reviewing how the code works, but 2 comments are:

it only supports a block-device storage class such as ocs-storageclass-ceph-rbd (or iscsi?), since you are making an ext4 filesystem on top of the PV, and this restriction should be documented in docs/fio_distributed.md (it probably was a restriction of the previous version also).   Shared filesystems like ocs-storageclass-cephfs or nfs would not work with this at present, but perhaps this approach could be made to work with them at a later date?

Also, is there going to be any CI for this so that we know it works now and in the future?  Existing tests/test_crs/ won't cover it..  What could be done - write a tests/test_crs/fio_cnv_ocs.yaml that only gets run in tests/test_fiod.sh if CNV is installed and OCS is installed (easy to check).  I did that with tests/test_crs/valid_fiod_ocs_cache_drop.yaml .

Hey @bengland2, thanks for the review.

* it only supports a block-device storage class such as ocs-storageclass-ceph-rbd (or iscsi?), since you are making an ext4 filesystem on top of the PV, and this restriction should be documented in docs/fio_distributed.md (it probably was a restriction of the previous version also).   Shared filesystems like ocs-storageclass-cephfs or nfs would not work with this at present, but perhaps this approach could be made to work with them at a later date?

Ah good point! I will work on adding a FS-mode compatible option.

* Also, is there going to be any CI for this so that we know it works now and in the future?  Existing tests/test_crs/ won't cover it..  What could be done - write a tests/test_crs/fio_cnv_ocs.yaml that only gets run in tests/test_fiod.sh if CNV is installed and OCS is installed (easy to check).  I did that with tests/test_crs/valid_fiod_ocs_cache_drop.yaml .

We had a discussion w/ the scale team on this, the test structure may be changing in the near future so while we do want to add a vm test_cr here at that point, for now we will have e2e VM workload test coverage in the benchmark-runner CI -- will need to add fio to that once we get this PR merged.

@jtaleric @rsevilla87 this PR is (finally) ready if I could please get a review

@RobertKrawitz @ebattat  Could one of you please give this a final review?
I have tested all the bus type and disk options for VMs locally w/ the operator, next step will be adding a fio testcase to the runner CI (after the current cleanup effort).

Thanks for the reviews, fixes pushed and tested locally.

Needs this PR https://github.com/cloud-bulldozer/benchmark-operator/pull/711 to retest

Needs this PR #711 to retest

Thanks @chentex but I am already using:  default('quay.io/cloud-bulldozer/fio:latest')
I have been seeing the fio pod client complete & index results since your pip upgrade PR merged.

Needs this PR #711 to retest

Thanks @chentex but I am already using: default('quay.io/cloud-bulldozer/fio:latest') I have been seeing the fio pod client complete & index results since your pip upgrade PR merged.

Ohh now I see, for the e2e check to pass. thanks!

Hi @jeniferh can you rebase this PR, it should pull all the latest fixes

Thanks @chentex for the recent wrapper fixes, I'm still getting e2e failures on non-fio tests, your PR #715 may help there?


Update saving, faq, command summary fields of user guide

Codecov Report

Merging #33 (f66f448) into master (b229253) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #33   +/-   ##
=========================================
  Coverage     72.15%   72.15%
  Complexity      399      399
=========================================
  Files            70       70
  Lines          1232     1232
  Branches        125      125
=========================================
  Hits            889      889
  Misses          311      311
  Partials         32       32

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.


1657393: Add missing descriptions

Summary
We are adding descriptions to improve SEO, per 1657393. Suggestions are welcome, but keep in mind our 100-160 char range.
@jkoritzinsky - You're listed as author for these topics. Please review when you get a chance. Our SLA specifies 5 business days for review, but let me know if you need more time. (Also, let me know if you are not the right person to approve. We go by the github author tag.)
Fixes #1657393

#sign-off


Update cats-mtl-core to 0.6.0

Updates org.typelevel:cats-mtl-core from 0.5.0 to 0.6.0.
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention @scala-steward in the comments below.
Have a fantastic day writing Scala!

Ignore future updates
Add this to your .scala-steward.conf file to ignore future updates of this dependency:
updates.ignore = [{ groupId = "org.typelevel", artifactId = "cats-mtl-core" }]

labels: semver-minor

Fixed in #79


Fix type checking of instructions in clifford stuff

I assume this code was written before pragmas etc were introduced. Executable
code now contains non-application stuff (pragmas) and so must be careful with
types.
Fixes rigetti/pyquil#858.

Had a look round and don't see anymore places where the same fix needs applying.

This might also give unexpected behavior in re: https://github.com/rigetti/pyquil/issues/858 , since it strips the PRAGMAs and then doesn't also reattach them to the output program.
Thinking about it some, I'm not sure that this is possible; doesn't the benchmarking API return a list of gate indices, so that it's not even possible to interleave non-gate-set-instruction directives? It might be :cool: to allow RPCQ to emit a special reply that contains a warning to be raised with warn + an actual payload to be returned as the call result, so that the RPCQ server could lightly slap Marcus's wrist when he passes in a program like this.

Good points, @ecpeterson. I can imagine how an identity-preserving pragma would be useful, however.

For what it's worth, qubits will never be relabeled by this routine, the program that comes out is guaranteed to agree semantically with the program that goes in (+ conjugation), and it's generally not possible to do make such a program without doing a bit of rewriting. A passed-through PRAGMA would only have an effect when the output was then further routed to one of the more traditional quilc compilation routines, and it feels like a misuse to me here. I like the idea of raising a warning.

Something about this doesn't feel right, both the old and new.
This is fixing something, but it's also letting things go wrong. What if there are classical instructions, what if there are other things?
I feel like there should be a type check that requires the input to have some property. If you run across something that's neither a pragma nor a gate application, perhaps it shall error?
What do you guys think? @ecpeterson @notmgsk

I think installing such an error is a good idea.

@stylewarning do we need a word for that? Something like "semiprotoquil"

If this https://github.com/rigetti/rpcq/pull/45 gets in first, I'd also recommend installing a warn on the PRAGMA path.

What the status for this? Just curious.

@marcusps There are some review comments to be addressed.


chore: fix typos, add code of conduct link

What kind of change does this PR introduce?
typo fixes in test/readme.md, updates code of conduct link in CODE_OF_CONDUCT.md

Did you add tests for your changes?
not required

Does this PR introduce a breaking change?
no

What needs to be documented once your changes are merged?
nothing

fixes #10564

Thanks


Is there an e2e integration test on toy data?

Describe the bug
In doing a major refactor (e.g. switching to OmegaConf or hydra), it's not clear to me there is a full e2e integration test. What main(s) would be the best on which to test this?
Additional comments
I might be mistaken, but tests/ only contains unit tests. A full e2e test on the most common main method(s), on a toy dataset, could test many code paths and make sure that some refactor does as intended. (This came up because I wanted to try a hydra port but had no idea how to quickly test if there was breakage or some crazy MSE on the downstream score versus the expected.)
An unintended sideeffect is codecov will increase :)

Indeed having end-to-end tests on the methods themselves is something that we need. However, using a toy dataset is not something that can correctly evaluate all methods. A decent middle ground can probably be a subset of imagenet100 (let's say 10%) for a couple of epochs and check if the obtained results (accuracy and loss values) fall into a predefined range that we need to compute beforehand. What do you think @DonkeyShot21?

@vturrisi Yeah, toy didn't mean synthetic necessarily. A tiny imagenet100 or tiny MNIST. (I suggest MNIST just because there are so few labels that fewer instances might make more sense.)
I googled quickly but couldn't find any tiny image datasets. But maybe you are familiar with some.
According to your profiles, 10% of 4m55 second epochs is about 30 seconds, on a GPU. You might consider, for this e2e test, using a smaller model than a big resnet so you can do it on CPU.
BTW, if you can decide upon a simple spec (which data set, which main functions you want to try, etc), I'm happy to contribute to the development.
COOL NOTE: Lightning Ecosystem CI allows you to "automate issue discovery for your projects against Lightning nightly and releases.
You get CPUs, Multi-GPUs testing for free, and Slack notification alerts if issues arise!" Since you have over 500 stars, they will allow you to include solo-learn in their nightly CI and get access to their multigpus. (Is solo-learn multigpu enabled? I haven't poked around yet.)
I would suggest with writing simple e2e tests in your repo and then later adding them to the lightning nightly CI.

@turian
I think the most important tests would be to validate the performance of the methods (as the other features are easily tested by the unit tests). Linear evaluation is also very decoupled from the methods, so I'm not so concerned with it. If 15 epochs is enough, we would need around 2 hours to run the tests for all the methods (assuming a 10% subset of imagenet100).
I'm also not sure how to manage data with github actions such that we can upload this imagenet100 subset (is this even possible?). The first step would be to check if we can upload datasets and then run all the current methods in that specific setting to gather some range of values for their losses and top-1 acc values to write the tests.

@vturrisi
I think we're kinda talking about two separate things. I'm more interested in an e2e test that runs quickly and just makes sure nothing breaks. (Unit tests are cool but don't always test the handoff points between different units.)
You are interested in doing hardcore regression testing to make sure scores don't drop on a known dataset.
A few opinions on my e2e proposal:

Regardless of unit tests, major refactors tend to break the glue between units. So one primary goal of an e2e test is to make sure everything runs start to finish.
A full e2e test should be <= 20 minutes, otherwise you will tear your hair out waiting for everything to finish.
Imagenet100 is like 20GB? Yeah. My suggestion is find a tiny dataset like MNIST 10% or whatever that is 100MB and create a separate github repo for it. (Don't use zenodo, it's flaky.) Then your git action just wgets from your separate repo. And your final check is that on this tiny dataset you don't have a score regression.

FYI, Travis will offer free credits to academic / open source projects, but these get exhausted very quickly if you use huge testing matrices (like every python x every pytorch x every OS), so I'd use that judiciously and only as a periodic supplement to github actions. (Maybe every time something is merged to main, not every single push.)
Regarding your suggestion:

I like the idea of making sure that scores don't fluctuate much.
2 hours using GPUs or CPUs? If GPUs, it's hard to find CI/CD providers that give GPUs :
Lightning CI might be an option?
You probably want something automated. Running colab scripts occasionally is a pain. If you have lab GPUs you could set up a cronjob. Alternately, you can spin up AWS spot instances using spotty and that should cost about $1/hr. Still, these ideas require extra annoying tooling.
Lightning CI I think is cool too. We can talk to them about your needs and they might be the best solution overall. (I speak with them frequently too.)

Overall, my suggestion is get the simple dumb fast e2e test working first (as I described above). Once that works, we can figure out how to do a proper e2e regression test on a "real" data set.

For GPU/TPU testing, you might also consider asking CircleCI for a grant. I think that's what lightning uses, but I'm not sure. (They are commercial so of course they pay.)

But given the number of stars and cites for your paper, it seems to make sense.

@turian been quite busy this week, but I'll try to get back here as soon as possible. Regardless, the end to end tests that you mentioned can be easily done with cifar10 and even without gpu with GitHub actions. It's just a matter of defining the scripts in a similar way to what I did in tests/args/test_args.py, e.g. just define the scripts as strings, save them and call a subprocess to execute it.
For the tests that I mentioned, I think we don't need anything fancy or automatic, just a set of scripts that we could manually run every couple of versions (or before any major version) to properly assess that nothing got screwed, performance wise.

The latest commit has tests for all scripts in tests/scripts. I think they are sufficient for the purpose of checking if there's something wrong with any script or method. About having performance tests, I'll try to address this in the near future.


Customize configuration files through configmaps (routr-connect Helm chart)

Is your feature request related to a problem? Please describe.
We're looking to deploy this as a subchart with another one containing Asterisk instances. Ideally this would be pre-configured as in once deployed Routr would loadbalance the asterisk instances automatically. However as far as I understand we need to add peers through the CLI first after deploying.
Describe the solution you'd like
If we could have parameters to define peers, credentials, etc I think they would be great additions to the chart.
Describe alternatives you've considered
Additional context
Configuration files: https://github.com/fonoster/routr/tree/3897ad320d98a0d17ca7dd552fc0ca7c5a84368d/config/resources

I'm converting this into a conversation. Although I believe we can improve the process of seeding the database, I don't think it belongs in the Helm chart. Perhaps we could introduce a command for rctl to seed the DB from a JSON file?


Error SignatureDoesNotMatch on Digitalocean(custom endpoint)

When invoking s3 (or other services) for digitalocean, but I expect the problem might extend to all other custom s3 backends, I am getting an error. The same invoke works without errors for jvm clojure aws-api.
Snippet included:
(def s3-client (aws/client {:api :s3
                            :endpoint-override
                            {:protocol :https
                             :hostname "fra1.digitaloceanspaces.com"}
                            :credentials-provider
                            (credentials/basic-credentials-provider
                             ;; secondary key
                              {:access-key-id     "XXX"
                               :secret-access-key "XXX"})}))

(aws/invoke s3-client {:op :ListBuckets})

{:Error {:Code "SignatureDoesNotMatch",
         :CodeAttrs {},
         :HostId "2b9e7d16-fra1a-fra1",
         :HostIdAttrs {},
         :RequestId "tx00000000000000a6c056b-0063685958-2b9e7d16-fra1a",
         :RequestIdAttrs {}},
 :ErrorAttrs {},
 :cognitect.anomalies/category :cognitect.anomalies/forbidden}

Having DO issues with cognitect's api is not new. For a brief period the steps here were working for me.
aws-api issue 150
It no longer does. I've since moved away from using DO spaces, but the issue linked is a great starting point.

Closing as this does not work in aws-api either, and fixing it would likely require an additional divergence from the aws-api codebase.


Singleshare is giving error on android if twitter is not installed

Hi,
When I am trying to do singleshare via twitter and If app is not installed on my phone it is giving following error on android

Same issue for Facebook... How can I handle it??....

Getting this for Twitter / Facebook / Google+ at least..

+1

same issue. I think, it should open web to share.

I also had this issue.  Was able to get it resolved by making a couple of edits in the MainApplication.java file (I think)?  Just closed the issue this morning. See here https://github.com/EstebanFuentealba/react-native-share/issues/229

The error is gone now. But if Twitter is not present it should open web to share right? But it opens google photo and gallery to share. 👎


Use records for properties/attributes

With RowToList we use records rather than the current property arrays in the HTML DSL.

I think we'd need compiler optimizations to remove dictionaries before this is practical. Using RowToList for properties will generate a lot of garbage dictionaries that have no actual evidence.


Suppress button not working

Hi
I am trying to suppress false possitives on my application, but the suppress button doesnt seem to work. When clicking on it, no dialog box or similar is displayed
I am using version 1.4.3
Thanks

What browser/version are you using?
On Nov 4, 2016 11:10 AM, "javixeneize"<EMAIL_ADDRESS>wrote:

Hi
I am trying to suppress false possitives on my application, but the
suppress button doesnt seem to work. When clicking on it, no dialog box or
similar is displayed
I am using version 1.4.3
Thanks
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jeremylong/DependencyCheck/issues/609, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AA0qwlW820C2pvbK7AEElXn5e4NOV6VPks5q60rjgaJpZM4KppRk
.

I have tried it with firefox, iexplore and chrome. Inspecting the button i
can see the copy action, but nothing is copied
As its the first time i am using this, i also tried the complete xml doc,
and nothing happens there too
On Fri, Nov 4, 2016 at 4:23 PM, Jeremy Long<EMAIL_ADDRESS>wrote:

What browser/version are you using?
On Nov 4, 2016 11:10 AM, "javixeneize"<EMAIL_ADDRESS>wrote:

Hi
I am trying to suppress false possitives on my application, but the
suppress button doesnt seem to work. When clicking on it, no dialog box
or
similar is displayed
I am using version 1.4.3
Thanks
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jeremylong/DependencyCheck/issues/609, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/
AA0qwlW820C2pvbK7AEElXn5e4NOV6VPks5q60rjgaJpZM4KppRk>
.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/jeremylong/DependencyCheck/issues/609#issuecomment-258478390,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMK55i5niMmd2nArZxfLjC-fSAhV9YrPks5q61v6gaJpZM4KppRk
.

If you're viewing the dependency-check HTML report from a Jenkins workspace, then this is expected behavior as Jenkins introduced CSP filters that prevent inline Javascript from executing. Workaround is to save the file locally and view it that way or relax the CSP filter.

Thanks Steve. Will check that
What should happen when i do this? Will it generate a xml?
If so, maybe its easier to create that xml manually
Let me explain the issue i have and what i want to achieve
I have a job that updates the vulnerability DB every day
I have 6 jobs that read from that DB and analyse my files. In all those
jobs i have the same false possitive.
I want to remove this false possitive to make from all the jobs. Do i need
to do that one by one?
Note that i am deleting the workspace before running my jobs, so if the
false possitive file is stored in this directory, it will be removed when
the job runs
thanks
On Fri, Nov 4, 2016 at 4:45 PM, Steve Springett<EMAIL_ADDRESS>wrote:

If you're viewing the dependency-check HTML report from a Jenkins
workspace, then this is expected behavior as Jenkins introduced CSP filters
that prevent inline Javascript from executing. Workaround is to save the
file locally and view it that way or relax the CSP filter.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/jeremylong/DependencyCheck/issues/609#issuecomment-258484324,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMK55p7D_E8tDl0HM0RV18Vg9TnUwfmsks5q62EXgaJpZM4KppRk
.

If you're scanning with the Jenkins plugin, the XML will always be generated. The HTML report can optionally be generated and like I said, the CSP policy in newer Jenkins versions breaks the suppress button and many other things.
Ensure that each of the 6 jobs have "Disable NVD auto-update" checked.
You can host the suppression file on a web server and instead of supplying the path to the file, simply specify the URL. This allows you to have a single suppression file in a global scanning environment.

@stevespringett To solve the CSP issue within Jenkins we could create a slightly customized report for Jenkins. Where we put a copy of the JS into a second file that gets written to disk, leave the inline script so that the report is still viable (when loaded from Jenkins the CSP would block this), and add a script tag to load the external JS. We could likely get the report to work - might have to think about the JS being loaded twice in some cases if the report and script are outside of Jenkins.
Thoughts?

Maybe theres an easier option. Who will need to read that exclussion list?
Why dont just create a properties file?
I am an owasp member. I am happy to help with this if needed
Another thing i have noticed is false possitives based on the name. Maybe
an md5 validation will solve this. Do you want me to raise another issue
for that?
Apart from that, guys, the tools is fantastic. Well done!
El sábado, 5 de noviembre de 2016, Jeremy Long<EMAIL_ADDRESS>escribió:

@stevespringett https://github.com/stevespringett To solve the CSP
issue within Jenkins we could create a slightly customized report for
Jenkins. Where we put a copy of the JS into a second file that gets written
to disk, leave the inline script so that the report is still viable (when
loaded from Jenkins the CSP would block this), and add a script tag to load
the external JS. We could likely get the report to work - might have to
think about the JS being loaded twice in some cases if the report and
script are outside of Jenkins.
Thoughts?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/jeremylong/DependencyCheck/issues/609#issuecomment-258602440,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMK55jiJ3VuU0GANVqEdk950G_r7OTIpks5q7FY-gaJpZM4KppRk
.

Doing a little more research on the CSP set by Jenkins the only way to make the HTML report work correctly (being served from Jenkins) would be to modify the CSP header by following the instructions in the article linked to above.
As @stevespringett provided the correct answer, host the suppression file on a different server, and there is nothing we can do regarding the CSP header I am going to close this issue.
Steve - the only thing we may want to do is provide a link on the Jenkins wiki pointing back to the CSP article above.

Hi
I have found a workaround that doesnt need to modify jenkins. Its just
editing the served page and removing the CSP headers ;)
Based on my experience, i can recommend one improvement
Looks like the tool is just detecting the name and the version. If there
is a vulnerabilty in apache 1.4, and i create a jar called apache1.1, the
plugin tells me that this jar is vulnerable. I am not sure if a MD5 check
could be added to reduce the number of false possitives
On top of that, instructions for the exclussion xml are a bit unclear. It
only explains how to generate the exclussion xml, but there are no
instructions about how to reference and configure it, etc..
Thanks
On Sat, Nov 12, 2016 at 3:59 PM, Jeremy Long<EMAIL_ADDRESS>wrote:

Doing a little more research on the CSP set by Jenkins
https://wiki.jenkins-ci.org/display/JENKINS/Configuring+Content+Security+Policy
the only way to make the HTML report work correctly (being served from
Jenkins) would be to modify the CSP header by following the instructions in
the article linked to above.
As @stevespringett https://github.com/stevespringett provided the
correct answer, host the suppression file on a different server, and there
is nothing we can do regarding the CSP header I am going to close this
issue.
Steve - the only thing we may want to do is provide a link on the Jenkins
wiki pointing back to the CSP article above.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/jeremylong/DependencyCheck/issues/609#issuecomment-260130235,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AMK55pMZEnQ2WZGsilTKDRuYztg7G-IQks5q9eJEgaJpZM4KppRk
.


PurpurMC Loud static / glitchy noise through bot

Describe the bug
Users speaking to eachother through the mod in game, and users speaking to the discord bot all produce clear audio. The issue is that any audio the bot forwards to the voice channel has very loud static and cuts out frequently
To Reproduce
Steps to reproduce the behavior:

Setup server with latest version of this plugin, simple voice chat, and PurpurMC
Run /dvc start
Talk to person through the mod / discord bot
Very loud static and glitchy audio coming from the bot itself
Expected behavior
A clear and concise description of what you expected to happen.
Expected clear enough audio both ways
Screenshots
If applicable, add screenshots to help explain your problem.

Logs
First, set debug_level to 3 (or 2 if there's too much spam) in the Simple Voice Chat Discord Bridge configuration file. After restarting your server or using /dvc reloadconfig, please upload the server logs here (make sure they don't include any sensitive info).
(log since /dvc reloadconfig for log level 2)
https://mclo.gs/lRSx0ao
Versions (please complete the following information):

Minecraft: 1.20.1
Loader (paper, purpur, fabric, quilt, etc): purpur
Simple Voice Chat: latest
Simple Voice Chat Discord Bridge: latest

Additional context
Add any other context about the problem here.

This was reported to me via DMs also. Not sure why I never encountered it during testing, I'll have to look into it on the weekend or something

Could you provide logs with debug_level set to 3?

Should be fixed with 2.0.1


How to set the http port of microservice(s) when usig redis transport

Hi 👋
I use a hosting solution that allows http port only on 8080 and I would like to know how to "force" the http port of a microservice when using redis transport
my service is like that:
const seneca = require('seneca')({
  transport: {
    redis: {
      url: process.env.REDIS_URL
    }
  }
})

function yo(options) {
  this.add({role: "hello", cmd: "yo"}, (message, reply) => {
    reply(null, {answer: ">>>yo!!!"})
  })
}

seneca
  .use(yo)
  .use('seneca-redis-transport')
  .listen()

and I would like to write something like that:
const seneca = require('seneca')({
  transport: {
    redis: {
      url: process.env.REDIS_URL
    },
    http: { port: 8080 }
  }
})

or like this:
seneca
  .use(yo)
  .use('seneca-redis-transport')
  .listen({
    host: '<IP_ADDRESS>',
    port: 8080
  })

thank you for your help 😄

If I try something like that with the microservice:
const seneca = require('seneca')({
  transport: {
    redis: {
      url: process.env.REDIS_URL
    },
    web: { type: 'web', host: '<IP_ADDRESS>', port: 8080},
    tcp: { type: 'tcp', host: '<IP_ADDRESS>', port: 8080}
  }
})

I can call it like that: http://yoservicedemo.cleverapps.io/act?role=hello&cmd=yo
and it's ok
but my client try to query http://<IP_ADDRESS>:10101
let clientYo = seneca({
  transport: {
    redis: {
      url: process.env.REDIS_URL
    }
  }
}).client()

  clientYo.act({role: "hello", cmd: "yo"}, (err, item) => {
    console.log(item)
  })

I get something like that:
["client","invalid_origin",{"pg":"","type":"web","id":"pg:","role":"transport","hook":"client","plugin$":{"name":"client$"},"fatal$":true,"meta$":{"id":"0v8rlakxn9tq/zkw424g3jjru","tx":"zkw424g3jjru","pattern":"hook:client,role:transport,type:web","action":"(iaboyz0bhpyf)","plugin_name":"transport","plugin_tag":"-","prior":{"chain":[],"entry":true,"depth":0},"start":1484492508558,"sync":true},"tx$":"zkw424g3jjru","port":10101,"host":"<IP_ADDRESS>","path":"/act","protocol":"http","timeout":5555,"max_listen_attempts":11,"attempt_delay":222,"serverOptions":{}},{"kind":"res","res":null,"error":{"code":"ECONNREFUSED","errno":"ECONNREFUSED","syscall":"connect","address":"<IP_ADDRESS>","port":10101,"trace":[{"method":"POST","url":"http://<IP_ADDRESS>:10101/act"}],"isBoom":true,"isServer":true,"data":null,"output":{"statusCode":502,"payload":{"statusCode":502,"error":"Bad Gateway","message":"connect ECONNREFUSED <IP_ADDRESS>:10101"},"headers":{}}},"sync":true,"time":{"client_recv":1484492547152}}]

Ouch! I think I found why it's ko

My hosting solution wait for a web app that listen on 8080, but map on 80
And the client read 8080 instead of 80 in the redis db
😢


Catch duplicate dynamic keys in dictionary

From #2559 is sounds the current inability to catch duplicate dynamically-evaluated keys was acquiesced as an unintended limitation. This patch adds checks at code evaluation that builds the dictionary.
The modified test in dict.typ showcased the changed behavior.
Please advise if accepting duplicate dynamic keys is actually really intended..? I think if users wanted to update the value corresponding to an existing key, they should explicitly do so with at() (as an lvalue) or insert(), instead of utilizing this undocumented "arcana".

I believe this comes from the fact that (some: thing, ..dictA, ..dictB) is a common pattern to override keys in a dictionary. You may get duplicate keys, but the rightmost key wins. So this is (at least) consistent with that pattern.
Regarding whether or not it should exist in this case, well, while it is forbidden when specifying keys manually (and I think that makes a lot of sense), I guess it should be fine (and less obviously a mistake) to override keys through dynamically evaluated keys. Thus this behavior can probably be rather convenient, so you don't have to (some: thing, ..((var): x), ..((other var): y)).

I believe this comes from the fact that (some: thing, ..dictA, ..dictB) is a common pattern to override keys in a dictionary

Makes sense! But this patch still allows this usage, and I added a test case to verify that:
#let other = (a: 2, "foo": 2, "bar": 2)
#test((a: 1, foo: 1, ("bark".slice(0,3)): 1, ..other), (a: 2, foo: 2, "bar": 2))

It doesn't work the other way around though, which I think is just as valid a use case:
#(..(a: 2, b: 3), a: 1)

The choice to not fail on duplicate keys was intentional judging from my comment there (I can't remember):

From my testing, it does not catch conflicts in evaluation. However, I believe that is correct behaviour. With static keys, a conflict is always an error, but with dynamic keys it might only arise in certain program paths and just taking the value of the last key is fine imo.

There are ways to allow spread (..other, a: 1) or (a: 1, ..other). If this "spread" case is allowed to override keys, is it a good a idea to disallow duplicate keys?

Which cases would remain that you would disallow?

Which cases would remain that you would disallow?

The examples in the dict.typ test before this PR. All of them doesn't involve spread.

Okay. I think I still prefer just overwriting things. That seems like reasonable behaviour in the general case, where raising an error might be surprising and only happen for certain inputs of a general function.


.Net: Plan for Models As A Service AI Connector

Create an ADR defining the scope of work
Create a task breakdown

Currently supported:

Chat Completion
Embedding generation

Will be supported:

Image generation

May not be supported:

Text completion

Naming of the connector:

Azure
AzureAI
AzureAIInference
...

Tasks created:

#7957
#7958


[playground] remove double item number

If you want to let customTitles have a # that will be replaced. otherwise we need to remove the .replace instead


Animate on update

I'm looking for the best way to update a doughnut chart, without it animating from scratch, but rather from the position is was in. For example, we're looking at a live pie chart showing the ages of people in a room, but when we update the chart, so every 5 seconds, we need to shuffle.
Looks and works perfectly without animation, but we're keen to get the easing there, etc. Any ideas?

Closing issue as abandoned. There have been several releases since this version and there is no test case.
If anyone is still seeing this issue with the latest version, please create a test case using JSBIN and reopen this issue.


BUG: Docker API access issues in local env

Description

Users are experiencing issues accessing the API when running the application through Docker, despite having the proper environment configuration.
The API endpoints at localhost:8080 are not accessible.

Current Configuration

Environment includes API_ENABLED=true in .env file
Running with docker-compose up -d

Observed Behavior

API endpoints are not accessible at localhost:8080
Frontend can start but cannot make API calls

Steps to Reproduce

Set up Docker environment with provided configuration
Include API_ENABLED=true in .env file
Run docker-compose up -d
Attempt to access API endpoints at localhost:8080

Impact

Blocking local development setup for team members
Similar issues reported by customer attempting integration

I'm not yet done the setup to run the Oracle so I can't verify this, but this sounds like something that I've seen on certain Macs. The Docker daemon runs inside a VM called Colima, which by default does not expose any ports to the host. You need to do the following:
$ docker-compose down
$ colima stop
$ colima start --network-address
$ docker-compose up -d

While this will still not expose the port on localhost, it will assign to the VM an IP address that is visible from the host. You can find out the IP address by running colima list. In my Mac it's <IP_ADDRESS>, so the API should be available at <IP_ADDRESS>:8080.
Reference: StackOverflow.

I've done further testing. As long as you do the colima start --network-address bit you should be able to connect to localhost. You only need to do it once, the setting is persistent.
If this happens on something other than a Mac, then I have no idea.


Video to be replaced | Gresa fixes the final video

Just upload on mute, can you? :-D
https://user-images.githubusercontent.com/47977202/199546583-d78384ed-bbc0-4130-8b90-a2011d3c2da8.mp4

Posted by @valmirmustafa:

Just upload on mute, can you? :-D
https://user-images.githubusercontent.com/47977202/199546583-d78384ed-bbc0-4130-8b90-a2011d3c2da8.mp4

Closed with d81e61650e46898431725137a9a62c6542d9675d


[2.0-beta12] error loading main.js and commons.js

I have been working to port an existing app to 2.0 (both to test it out and to just casually begin porting to it as it's developed) and noticed the following issue (also replicated on the custom server example).
The page loads via SSR but throws a 500 error when calling the js bundles http://localhost:3000/_next/commons.js and http://localhost:3000/_next/main.js
This happens when running Node: 6.0.0... I have not tested other node versions

TypeError: Cannot read property 'R_OK' of undefined
TypeError: Cannot read property 'R_OK' of undefined
    at _callee6$ (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/next/dist/server/render.js:283:72)
    at tryCatch (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/regenerator-runtime/runtime.js:64:40)
    at GeneratorFunctionPrototype.invoke [as _invoke] (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/regenerator-runtime/runtime.js:355:22)
    at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/regenerator-runtime/runtime.js:116:21)
    at step (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/babel-runtime/helpers/asyncToGenerator.js:17:30)
    at /Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/babel-runtime/helpers/asyncToGenerator.js:35:14
    at new Promise (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/core-js/library/modules/es6.promise.js:191:7)
    at /Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/babel-runtime/helpers/asyncToGenerator.js:14:12
    at serveStaticWithGzip (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/next/dist/server/render.js:313:19)
    at Server._callee5$ (/Users/mattapperson/Development/Theocratic/kh-mirror-web/node_modules/next/dist/server/index.js:222:58)

here also!


Illustrate how to use the expertsystem with tensorwaves

With https://github.com/ComPWA/expertsystem/commit/1d822590d42cfdae2af380f766867aa1b3140c46, the expertsystem can write amplitude models for the helicity formalism to a YAML format that tensorwaves understand. So now we run tensorwaves over arbitrary decays with the helicity formalism (so far we just use one intensity-recipe.yaml file as an example/benchmark).
Note: there are tiny improvements to the YAML recipe file format, so that will have to be fixed in tensorwaves.

Best to first merge https://github.com/ComPWA/expertsystem/pull/51 so that we can create a release that has a better README, see https://github.com/ComPWA/expertsystem/pull/51#issue-430928105


Pain Point: Deeply Nested Amends }}}}}}}}}

I'm testing Pkl as a replacement for k8s tools like Kustomize/Helm. So far it's quite nice, but one pain point is modifying deeply nested properties.
As an example, say I have a HomeAssistant pkl package with a default volume claim of 1Gi, which I'd like to bump up to 2Gi.
import "ha-app.pkl" as homeassistant // extends AppEnvCluster, sets isLeafModule() = true
// assume the above is a third-party package

// default HomeAssistant application, with one modified property
local ha_app = (homeassistant) {
  statefulSets {
    ["core"] {
      spec {
        volumeClaimTemplates {
          [[metadata.name == "homeassistant-config"]] {
            spec {
              resources {
                requests {
                  ["storage"] = "2Gi"
                } // 9 lines of brackets :')
              }
            }
          }
        }
      }
    }
  }
}

output {
  files { // for use by argocd
    ["homeassistant.yaml"] = ha_app.output
  }
}

18 LoC for a single deeply nested amendment seems a little excessive.
One way I've found to improve this is to remove some newlines, but I don't find this ideal.
// I guess this is better?
local ha_app = (homeassistant) { statefulSets { ["core"] { spec { volumeClaimTemplates {
  [[metadata.name == "homeassistant-config"]] { spec { resources { requests {
    ["storage"] = "2Gi"
  }}}}
}}}}}

I could create late-binding variables in a new pkl module which extends ha-app further, but that seems like I'd just be moving the problem down the line.

Personally I'd like to see dot syntax supported for amendments. In my case, that might look something like:
local ha_app = (homeassistant) {
  statefulSets["core"].spec.volumeClaimTemplates {
    [[metadata.name == "homeassistant-config"]] {
      spec.resources.requests["storage"] = "2Gi"
    }
  }
}

Am I missing a more idiomatic solution?

We'd like to add support for flat member syntax (the solution that you suggested) at some point. I definitely agree that this a pain point.

That's great to hear, and thanks for the quick response! Feel free to change the title to something more suitable for tracking flat member syntax.
Fwiw, Pkl has been fantastic for me, and I often find myself telling people how amazing it is :)
I'm hoping I can bring it into my day job soon 🤞

foo.bar.baz.qux {...} looks very similar to expressions foo.bar.baz.qux and (foo.bar.baz.qux) {...}, which complicates parsing and reading. I think a different syntax should be considered.

Actually, the (...) in object amends syntax disambiguates that entirely. A slightly trickier thing about flat member syntax is; what do you do with consecutive amends;
foo.bar.baz = 0
foo.bar.qux.quux = "hello"
foo.bar.qux.corge = "world"

Is that the same as
foo {
  bar {
    baz = 0
    qux {
      quux = "hello"
      corge = "world"
    }
  }
}

or
foo {
  bar {
    baz = 0
  }
} {
  bar {
    qux {
      quux = "hello"
    }
  }
} {
  bar {
    qux {
      corge = "world"
    }
  }
}

That has implications for what super means (and for performance, but that's a shortcoming of the current implementation - albeit non-trivial). There are reasons why you're currently not allowed to amend the same property twice. Flat member syntax makes that ambiguous, because the overlap may be partial.
Or... should that ambiguity should syntactically be disallowed and should this require that properties are aggregated in notation, i.e.
foo.bar {
  baz = 0
  qux {
    quux = "hello"
    corge = "world"
  }
}

In both of those cases, super.foo should have the same meaning, no? It means the parent foo value in the prototype chain. super by itself isn't a valid expression.
An interesting problem here is what outer should mean. It would be surprising if these two were the same:
foo.bar.baz = outer.qux

foo {
  bar {
    baz = outer.qux
  }
}

Actually, the (...) in object amends syntax disambiguates that entirely.

Distinguishing foo.bar.baz.qux {...} from foo.bar.baz.qux requires arbitrary lookahead. It’s also difficult to parse for a human, especially in the middle of a large program. The similarity with (foo.bar.baz.qux) {…} further complicates human parsing.

Distinguishing foo.bar.baz.qux {…} from foo.bar.baz.qux requires arbitrary lookahead. It’s also difficult to parse for a human, especially in the middle of a large program. The similarity with (foo.bar.baz.qux) {…} further complicates human parsing.

I see your point, but I'm not very convinced that this is a problem. This is unambiguous without needing lookahead:
foo.bar.baz.qux { ... }

This does require lookahead (because bar.baz.qux is a valid expression, and expressions are valid object members):
foo {
  bar.baz.qux { ... }
}

But generally I wouldn't expect to see code that looks like this. I would more expect them to be flattened at the top level, as per the first snippet in my post.

The lookahead here seems easy to me.

Arbitrary lookahead can cause problems with syntax highlighters and parser tools. It can also cause performance issues. It’s best avoided if possible.
https://github.com/elves/elvish/issues/664

This is unambiguous without needing lookahead:

I think that’s only the case because typed objects can’t have elements and entries. Is this limitation here to stay?

I would more expect them to be flattened at the top level, as per the first snippet in my post.

I think it’s best to avoid such assumptions. If the feature can be used inside objects, it will be used inside objects.

I think that’s only the case because typed objects can’t have elements and entries. Is this limitation here to stay?

No, it's because expressions are not permitted at the module level. And, this isn't likely to change.

No, it's because expressions are not permitted at the module level.

But if typed objects could have elements, it would make a lot of sense to allow expressions (elements) at the module level. This would also work great for rendering formats that support top-level arrays (JSON, etc.).

Or... should that ambiguity should syntactically be disallowed and should this require that properties are aggregated in notation,

This might be a good first (and possibly last) step. Good editor support could ease the strictness pain.

But if typed objects could have elements, it would make a lot of sense to allow expressions (elements) at the module level. This would also work great for rendering formats that support top-level arrays (JSON, etc.). Same for entries.

Even if typed objects eventually allow entries and elements, I'm kind of bearish on this at the module level.
This starts to feel too much like a scripting language:
module foo

something()

somethingElse()

The way to render top-level arrays is to assign to output.value, e.g.
someListing: Listing<String> = new { "hello" }

output {
  value = someListing
  renderer = new JsonRenderer {}
}

Sounds like a simple expansion won't be good enough.

Yeah; we need to explore what the scoping rules should be. And, it might require that we start resolving variables at parse-time (within AstBuilder.java).


parse natural language

I'm working on s.th. where moment.duration() comes in handy, but my input is natural language.
while this works:
(moment().subtract( moment.duration({ hours:1 }) )).format();
"2015-03-09T17:04:11+01:00"

(moment().subtract( moment.duration("1:00") )).format();
"2015-03-09T17:04:11+01:00"

this doesn't:
(moment().subtract( moment.duration("1 hour") )).format();
"2015-03-09T18:04:11+01:00"

it would be nice if this was supported. a bit like later.js does it.

Hi @freder -- this has come up a few times, and we've consistently decided this kind of natural language parsing is outside the scope of Moment. It would make a great plugin, though!

It would still be very very nice if we could have english only parser like this: http://bunkat.github.io/later/parsers.html#text

Sugar.js has one.  Thus far, nobody has stepped up to write one for moment.

Well... they all modify native Date object globally and I had many issues because of that. That's one reason why I like moment.

Agreed it would be useful, but it is clearly out of scope for moment core.  A plugin would be welcome.  NLP is difficult, and requires dedication from someone who will maintain it.


Keep nodes from showing in any of the registration views

Purpose
Fixes [https://openscience.atlassian.net/browse/OSF-5491]. Ensures /registrations/ views require the node to be a registration rather than just catching it elsewhere
Changes

In get_node() for the registration mixin, verify that the node is a registration and raise a NotFound error otherwise.
Fix the test that was checking for that to the new way of saying the node is not available here
Add another test for a subview

Side effects
One (unrelated) test will still fail due to failing test in develop - fixed in #4844.

Build failure is unrelated to this PR and has since been fixed on develop. I've cherry-picked eb47501 into release/0.61.0. :octopus:


Make tqdm optional if manager is used as a python module.

If people use webdrivermanager as a python module, it would be nicer to let them decide if tqdm output can be disable in order to not mess up their console output.
Update: apparently i just didnt read the code carefully enough - it is a possibility already but there's a hard requirement for tqdm still present which is not nice from my perspective - might consider this to be pip install time extra feature ..

In the downloaders you have the param show_progress_bar which should disable the progress bar.
Example:
ChromeDriverManager().download_and_install(show_progress_bar=False)
This won't show the progress bar from tqdm


Documentation in readthedocs.

This change is

Codecov Report

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

@@           Coverage Diff           @@
##           master       #7   +/-   ##
=======================================
  Coverage   90.62%   90.62%
=======================================
  Files           9        9
  Lines         256      256
=======================================
  Hits          232      232
  Misses         24       24

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 3b0870b...a09992b. Read the comment docs.


[$250] Workspace - Approve button returns briefly for the admin after approving the expense

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

Version Number: 1.4.83-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4627303&group_by=cases:section_id&group_id=306206&group_order=asc
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: applause internal team
Slack conversation:
Action Performed:
Precondition: Create a Collect workspace in ND with an expensifail account. Invite a Gmail account as a member. Enable Workflows and add a Plaid Regions bank account.

Member: Navigate to https://staging.new.expensify.com/

Member: Log in

Admin: Log in

Member: Submit a manual expense to the workspace chat

Admin: Quickly open the IOU and approve it

Expected Result:
"Pay with Expensify" should be there until the expense is paid.
Actual Result:
"Pay with Expensify" button appears after approving, after a few seconds, it reverts to "Approve" and back to "Pay with Expensify".
Workaround:
unknown
Platforms:

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

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
https://github.com/Expensify/App/assets/43996225/8f7906fb-c568-419d-9c6a-99300e943b83

View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01ee0bafa7bb3dc0e3
Upwork Job ID:<PHONE_NUMBER>876368442
Last Price Increase: 2024-09-24
Automatic offers:

ZhenjaHorbach | Reviewer | 102863571

Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @techievivek

@stephanieelliott FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

We think this bug might be related to #wave-collect - Release 1

Waiting for proposals on this one

Hi, I'm Viktoryia from Callstack - expert contributor group - and I would like to work on this issue.

I wasn't able to reproduce the issue. Approve button doesn't return for me.
https://github.com/Expensify/App/assets/23176449/f50f7001-37a4-4f17-a929-10ee02586fe3

Note: I'll be OOO on Monday (24.06) 🌴

Thanks @ZhenjaHorbach! @VickyStash sounds like this is intermittent, maybe try reproing a few times?

I haven't had a lot of time today for this issue, but I was finally able to reproduce the issue.
It looks like the issue can be caused by a combination of OpenReport and ApproveMoneyRequest API calls.
So if a user quickly opens the expense quickly and press the Approve button, the following steps can happen:

The OpenReport API command is called when the expense is opened. In cases when I was able to reproduce this issue this request took 2+ seconds.
During OpenReport is processed, user taps on Approve button => local DB got updated with optimistic data for expense Approval. As a result the user see the Approve button turns into Pay.
The app gets the response of OpenReport API call, and the onyx data got updated with the data without approval information since API didn't know about the approval. As a result the user see the Pay button turns into Approve.
In the background ApproveMoneyRequest API call takes around 10-15 seconds. After that time the app gets the response of the Approval request and the onyx data got updated again with the correct data. As a result the user see the Approve button turns into Pay again.

I need some time to think about the solution.

Unfortunately, I haven't had enough time today to investigate this issue more, I'll do my best to take a look tomorrow.

Thanks @VickyStash! Glad we were at least able to nail down how to repro it

Note: I'll be OOO Jul 1 - Jul 7 🌴

@VickyStash is working on this. Given that it's hard to repro and is mostly a display issue that does not impact app functionality, we'll hold on this while they are OOO next week. Setting this to Weekly in the meantime since there will likely be no updates for several days.

Hi, I'm Eugene from Callstack - expert contributor group.
I have the capacity and am going to take over the issue

Great, thanks @pasyukevich -- I've assigned you on this issue!

Applying the New Feature label to get another BZ member on this while I am OOO til July 10. To catch you up on where we are @sonialiap, expert contributor @pasyukevich just took over this issue and is looking into it..
Thanks for watching over this, I'll grab it back from you when I return!

I was able to reproduce this behavior and am currently thinking about the solution

Today's update: Still working on possible solutions for this problem

Today's update:
As already mentioned, we have this issue only in the case of a delayed OpenReport Request during the opening of the expense page.
How this can be fixed:

Improve response speed on the API side for OpenReport API request
Remove optimistic data on the Approve request and add the loading state to the button

Also as I see we have flow with lastUpdateID to track orders of updates for Onyx. But this part does not apply to the optimistic data race condition with API update.
I think that this issue is also connected with this one - https://github.com/Expensify/App/issues/37560.
This issue looks global and can be reproducible in other places.
Should I prepare a proposal with a fix only for this case or focus on a global one?

Yeah @pasyukevich, if that is possible then I think we may as well go for a global fix rather than just fixing this case.

Got it, I will switch back to this issue tomorrow

Thanks @pasyukevich!

Today's update:
Work in progress, preparing the proposal

Proposal
Please re-state the problem that we are trying to solve in this issue.
When we open the freshly submitted expense from the collect workspace and press the "Approve"  button quickly, we can see that the state of this button will change to "Pay", "Approve" and "Pay"  back.
What is the root cause of that problem?
When we receive a Response from the "Approve" request straight after the OpenReport response we do not apply it and pause the queue as we detect inside the SaveResponseInOnyx that the previousUpdateID from the response is greater than lastUpdateIDFromClient. Because of this we are put on hold update and waiting additionaly for the getMissingOnyxUpdates request. In the middle of it, we are applying a response from the OpenReportRequest received just a moment before the ApproveMoneyRequest  response. Only after the getMissingOnyxUpdates request is resolved, we apply the data from the ApproveMoneyRequest response.
What changes do you think we should make in order to solve the problem?
On the API side, we need to have previousUpdateID inside the OpenReport request. In this case, we will detect the gap between the server and frontend state before showing the "Approve" button. User will be able to send ApproveMoneyRequestrequest only if the gap is eliminated
What alternative solutions did you explore? (Optional)
Change SaveResponseInOnyx method and even if we detected the gap between FE state and BE state apply a response for the current request, then put the queue on hold, and run getMissingOnyxUpdates request.

@stephanieelliott Can we have a review of the proposal from the internal team as @ZhenjaHorbach mentioned?
One of the proposals relates to the API side

Sure! @techievivek would you mind reviewing this proposal: https://github.com/Expensify/App/issues/43783#issuecomment-2252404214

This one certainly is a tricky one, reviewing the GH to see how we can move forward here, thanks.

I'm thinking of closing this dupe in favour of this issue as we've progressed further here. That said, can we confirm this will solve for the Submit button on delayed submission, referenced here. I can't see why not, seems to be the same bug.
@techievivek please also review the proposal so we can move it on. ;)

@danieldoglas @chrispader (I believe Chris worked on part of this) would you be able to check out the proposal too, I am not 100% sure if this is safe to do

Change SaveResponseInOnyx method and even if we detected the gap between FE state and BE state apply a response for the current request, then put the queue on hold, and run getMissingOnyxUpdates request.

I do not know any specific problem with this but just think there is a reason why its built the way its now

@techievivek please also review https://github.com/Expensify/App/issues/43783#issuecomment-2252404214 so we can move it on. ;)

I looked through the proposal, but both solutions need verification from experts. Vit has already tagged Daniel, so I will wait for them to review it and share their thoughts.
Also ccing @iwiznia and @tgolen since this change is related to Onyx updates. Thanks.

@techievivek I think we need to investigate the method that is being called for approving the report. It's probably not returning all the onyxUpdates it should, which then generates the gap for the next request.

I don't have any immediate thoughts, but I am intently following along!

Change SaveResponseInOnyx method and even if we detected the gap between FE state and BE state apply a response for the current request, then put the queue on hold, and run getMissingOnyxUpdates request.

I do not know any specific problem with this but just think there is a reason why its built the way its now

The reason this was done is so that everything happens to the client in the right order. If there is a gap of Onyx updates, that gap should be applied before the updates in the current response, or else the updates in the gap might overwrite the changes in the current response.

Tim is right and we should not change that behavior. It seems that in this flow we always go through the getMissingOnyxUpdates flow? If so what we need to do is figure out why that happens, as that should be rare and only happen when you missed an onyx update.

@techievivek have you been able to investigate this suggestion?

@techievivek I think we need to investigate the method that is being called for approving the report. It's probably not returning all the onyxUpdates it should, which then generates the gap for the next request.

No, I haven't been able to triage this again, I will take a look into it by tomorrow.

I looked through the request, and it seems like some updates are missing in the response. As a result, we call GetMissingOnyxMessages to fetch those updates.

Yeah that sounds wrong, can you debut it further to figure out why that happens?

Hey @techievivek have you had a chance to debug any further on this one?

No updates from last week. I will debug this further this week.

Thanks, that would be great because this is pretty janky in a mainline core flow. 👍

Just a note that this issue is linked to an external deadline of Sept 9th, if you can try and prioritize it soon @techievivek!

Hey, I have a few other similar priority issue on my plate, can we look for a volunteer here, thanks. 🙇

I'm currently handling two Fast-API issues: #419963 and #408736. Additionally, there's a cleanup task related to these issues that needs to be prioritized to fully utilize the improved performance: #427142.

There is another wave-control issue on my plate where we are going back and forth with the reviews https://github.com/Expensify/Auth/pull/11996

Thanks!

I'm currently handling two Fast-API issues: https://github.com/Expensify/Expensify/issues/419963 and https://github.com/Expensify/Expensify/issues/408736.

Both of these are LOW and HIGH in #fast-apis, which is below CRITICAL where this issue is classified in #wave-collect as we focus on bugs in core flows as the deliverable for the SuiteWorld release.

There is another wave-control issue on my plate where we are going back and forth with the reviews https://github.com/Expensify/Auth/pull/11996

That one looks like it's attached to the Violations in Auth project which is classified as HIGH in the OP tracker, and #wave-control list of projects in the channel here.
So I would suggest this issue takes priority 👍

sorry did not mean to touch the label here. readding!

@techievivek just wondering if you had any thoughts on my comment here: https://github.com/Expensify/App/issues/43783#issuecomment-2340272990

@techievivek another bump on this -- the two Fast-API issues you referenced were closed yesterday, can we bump the priority of this one now?

Yes, it's now a top priority for me. Since I'm still getting familiar with the flow, it's taking a bit longer, I will update the GH today.

Hmm, I can't reproduce this anymore. I'm going to ask QA if it's still reproducible.

@kavimuru

The app has changed a bit, now tester only have the pay button without additional setup, but tester can still repro the issue with the pay button.

I think you will need to be on control policy and have the approval flow set so that when we submit an expense above a certain amount, admins need to manually approve it before paying.

@trjExpensify Are you able to reproduce this? I have been trying to reproduce this since yesterday but have had no luck. Am I holding anything wrong here?
https://github.com/user-attachments/assets/2fb4409a-2205-46f2-bab2-adee4f4c1189

CC @stephanieelliott can you also give it a look and see if you can reproduce it? Thanks.

@techievivek The app has changed a bit, now tester only have the pay button without additional setup, but tester can still repro the issue with the pay button.

QA repro'd 18 hours ago in that video? Let's confirm their setup.

@techievivek did you confirm the setup in Slack?

Hey @techievivek, what's the latest here?

I have been ooo for the last four working days. I'm catching up, but updating here that I haven't yet confirmed the setup used by QA on Slack.

Okay, well the first diff I see in your video here versus the prerequisite steps in the OP (copied below), and QA's video here, is that it doesn't look like you're testing with a VBBA connected to the workspace. I'd start there.

Precondition: Create a Collect workspace in ND with an expensifail account. Invite a Gmail account as a member. Enable Workflows and add a Plaid Regions bank account.

I tested this again following Tom's comment above. While I couldn’t reproduce the original issue mentioned in the GH OP https://github.com/Expensify/App/issues/43783#issue-2353765649, I was able to replicate the problem Applause highlighted here: https://github.com/Expensify/App/issues/43783#issuecomment-2353526195. Specifically, I had to click 'Pay with Expensify' twice for the payment to go through, which is definitely a bug.

https://github.com/user-attachments/assets/065b4bdc-b469-4da9-864c-da388d1358e9

Great! It would be ideal to replicate the exact conditions to rule those out for the approve button flash - so don't have dupes to cause Hold to be another variable in the mix, and per the OP, the invited member is a gmail account.
Either way, agree the Pay button shouldn't need to be clicked twice. Also, did you keep an eye on the LHN, see how that preview messages goes from approved > paid > approved > paid?
https://github.com/user-attachments/assets/c2688fd7-ef95-480a-a592-6116f99743f5

@techievivek Is there anything that I can help with?
My initial proposal - https://github.com/Expensify/App/issues/43783#issuecomment-2252404214

Hey @techievivek any update? Would it be possible leverage external contributors to help with a fix? @pasyukevich has mentioned they are willing to help

This is more of a backend issue, and from my last investigation https://github.com/Expensify/App/issues/43783#issuecomment-2396796522, it seems like we are creating duplicated actions for some reason. I couldn't dive into it further due to fireroom related priorities but I will give it a look today.

Issue not reproducible during KI retests. (First week)

@trjExpensify I think we can close this and I can rather be assigned to the other GH where the new version of the problem is visible(double tap on pay button).

Where is the other GH?

@techievivek are you talking about https://github.com/Expensify/App/issues/45624?

I think @techievivek is referring to this issue: https://github.com/Expensify/App/issues/47503#issuecomment-2413546487

Hey @techievivek is https://github.com/Expensify/App/issues/47503#issuecomment-2413546487 the one you're referring to? Anyone disagree with consolidating to that issue and closing this one?

The original problem mentioned in this GH issue is no longer reproducible. We're now facing a different issue, which you can find here: https://github.com/Expensify/App/issues/47503#issuecomment-2413546487(same as what you mentioned). I think we can close this issue and I can assign myself to the new one, as I already have some context on it.

Sounds good, thanks @techievivek!


Swift 3 API Parity: Drop NS prefix from NSHTTPURLResponse

The Swift 3 API changes dropped the NS prefix from NSHTTPURLResponse, but that was missed in the URLSession code drop.
This remove the NS prefix from the declaration and where its used in both Foundation and TestFoundation.

@swift-ci please test linux

@parkera Is this good to merge? It looks like there might be some other API changes required, so it would be good to get this merged to avoid any change conflicts.

Yup, thanks.


Added moving car animation

*Fixed #629

🛠 Fixes Issue #629

👨💻 Changes proposed

✅ Check List (Check all the applicable boxes)

[x] My code doesn't break any part of the project (Zero Octave-Javascript-Projects).
[x] This PR does not contain plagiarized content.
[x] My Addition/Changes works properly and matches the overall repo pattern.
[x] The title of my pull request is a short description of the requested changes.

📄 Note to reviewers
It is an animation of a moving car, with music feature build using HTML, CSS, JavaScript
📷 Screenshots

@Astrodevil  I have made a new PR for moving car animation, Please view it.

Still sound is not working

@Astrodevil There is some problem with the sound :( so I have removed it.


Add to LoggingError a property bag to hold telemetry props

Fixes #6485
Need to add/update tests.

title doesn't match the PR

Rethinking this, and needs to get some merges fixed up from recent related changes.  May reopen.


Introduce $FEDSCALE_HOME. Closes #97

Good point. But does .bashrc run when someone is in the interactive shell?

We can add a setup_commands to source ~./bashrc, I will have a try late this day

It does not work for....source ~./bashrc in the setup_commands😭


Forms merge in geo-compose

I'd like to merge geo-compose into the forms feature branch. The merge is quite painful due to android app and library projects added in each branch. git gets confused by all the identical files and thinks they have been moved. This will need to be done before release and I've got it done here for dev-summit publication.
There are also changes in both branches to the composable-map project, and to the CompassApp.

not merging in this way. closing.

do not delete branch please!


Add experimental support for big endian hosts (read part only)

This PR is not ready for getting pulled. I put it here only for further discussion. See also issue #518.
Writing training data on big endian machines is currently not addressed.

Yes, I think it would be possible to use templates in both cases.
I did not do that because I wanted to see which cases occur in practice. Some of those cases are problematic, because they don't use portable data types: the current code reads and writes enum and int data which happen to be 32 bit on Intel, but maybe not everywhere.


Add request_id field to some Create requests as per AIP-155.

This replaces all other idempotency key mechanisms. As a result, it is a backwards-incompatible change.

This change is


[Branch-2.8]fix testSetReplicatedSubscriptionStatus run failed

Motivation
The test testSetReplicatedSubscriptionStatus  add by #10790, and the code persistentTopics.createPartitionedTopic(response, testTenant, testNamespace, topicName, 2, true); createLocalTopicOnly set true. However, when the PR cherry-pick to branch-2.8, this commit https://github.com/apache/pulsar/commit/0706bc01a428a9405c671a0f80fb54faccdd26e6, which lead the test run failed.

Fix #11662

enable all CI tests for pull request.


Add trans filter to replicate Symfony/Drupal trans

We have a custom Drupal theme that uses Storybook.
When we load up Storybook, the trans filter in the twig PHP files throw errors.
Thus, we add a simple trans filter to twig.js to get around this issue.

The trans filter is not part of the core Twig specification. I think that it should not be put in twig.js code base, but should be provided by a third-party library.

@ericmorand do you have a suggestion on how that should be implemented?
We've been using a patched version of twig.js for a few years with the change from this PR, then we npm run build and npx patch-package twig to get our patch file for the current version of twig.js that we're using.
For our needs, it has been working fine, I figured I would just provide the solution in case others want to do something similar.

twig.js supports registering custom tags:
https://github.com/twigjs/twig.js/wiki/Extending-twig.js-With-Custom-Tags
Does it fit your need?

@dan2k3k4 There is also this: https://github.com/JohnAlbin/drupal-twig-extensions

Closing this since trans is not part of core Twig.


CloudStack 4.19.0 can't export templates/ISOs by CloudStack UI

All i want is to download templdates already made from instances.
CloudStack 4.19.0 is deployed and all the components work fine.
ISOs are uploaded successfully and Templates are converted from volumes successfully.

System vms are checked that they are running without anything unusual.

I try to export/download templates from CloudStack UI by the button 'download template' but failed.
wget/curl has been tried.
edge/chrome has been tried.
system vms has been restarted.

ISSUE TYPE

Bug Report

COMPONENT NAME

CloudStack UI, API

CLOUDSTACK VERSION

CloudStack 4.19.0

CONFIGURATION

basic network

OS / ENVIRONMENT

CentOS 7.9.2009

SUMMARY

Download templates from CloudStack failed.
STEPS TO REPRODUCE

EXPECTED RESULTS

templates are downloaded successfully.

ACTUAL RESULTS

failed to download templates with timeout.

@Dragonzz27 , I am not able to reproduce the behaviour
Could you please try with the extract template API call
https://cloudstack.apache.org/api/apidocs-4.19/apis/extractTemplate.html
Example
(CMK) > extract template id=b451ccef-0ff5-4702-bcd1-66e73fe48450 zoneid=7625c6bd-67ee-4847-a3b4-48ee73281aed  mode=HTTP_DOWNLOAD
{
  "template": {
    "accountid": "80c25e48-2941-11ef-b10c-1e007c000297",
    "extractMode": "HTTP_DOWNLOAD",
    "id": "b451ccef-0ff5-4702-bcd1-66e73fe48450",
    "name": "kube-v1.27.2/ubuntu-2004",
    "state": "DOWNLOAD_URL_CREATED",
    "url": "http://<IP_ADDRESS>/userdata/7ca93d49-d6e0-4cfa-a755-a0b5133bf93b.qcow2",
    "zoneid": "7625c6bd-67ee-4847-a3b4-48ee73281aed",
    "zonename": "ref-trl-6823-k-Mr8-kiran-chavala"
  }
}

Then try wget the file
╰─ wget "http://<IP_ADDRESS>/userdata/7ca93d49-d6e0-4cfa-a755-a0b5133bf93b.qcow2"
--2024-07-29 15:12:04--  http://<IP_ADDRESS>/userdata/7ca93d49-d6e0-4cfa-a755-a0b5133bf93b.qcow2
Connecting to <IP_ADDRESS>:80... connected.
HTTP request sent, awaiting response... 200 OK
Length:<PHONE_NUMBER> (4.1G)
Saving to: ‘7ca93d49-d6e0-4cfa-a755-a0b5133bf93b.qcow2’

7ca93d49-d6e0-4cfa-a755-a0b5133bf93b.qcow2       0%[                                                                                ]   1.15M  1.04MB/s

@Dragonzz27 Can you check the following:

Connectivity to the IP and port 80 of the URL.
Try from another client, network.
Check if the template is present in the secondary storage.

Really thanks to @kiranchavala and @rajujith , the problem has been solved and templates/ISOs can be downloaded successfully.
I checked two system vms following rajujith's suggestions and found that the consoleproxy vm could be pinged but secondarystorage vm could not be pinged successfully.
Although apache2 server in the vm looks working fine, but when curl http://IP_Address:80, it failed.
So I finally delete the secondarystorage vm and reinstall using the vmtemplate, with a result, I can download templates and ISOs using edge without any problem.


fix: catch possible pickling errors

Summary of Changes
Catch pickling errors in two other locations where they might occur.

:tada: This PR is included in version 0.17.0 :tada:
The release is available on:

v0.17.0
GitHub release

Your semantic-release bot :package::rocket:


Write README.md

It would be great if you can add more some info for each issue that make our team easy to understand the problem

It would be great if you can add more some info for each issue that make our team easy to understand the problem

Yes, that is a good idea so I will add related information to the next issues. thanks


Update Documentation/README for geopandas.plot_bokeh

Use for example New York Taxi data for examples or OSM data

Examples for polygon-geoplots are now online. Still missing are:

pointsplots (e.g. locations of Starbucks in the US)
lineplot (e.g. rivers or streets from OSM)


You can steal things from people's inventories using crafting

Description

If you are standing near a person with ingredients in their inventory, you can make things from the crafting menu (spears, tiles, pneumatic cannon, etc) using their ingredients. You can also just move to interrupt the craft and steal the materials.
Screenshots.

Additional context

Works through walls, from inside cloners, etc. Can pop out of a cloner with a spear and handcuffs ready to take down your oppressors.

Sounds like crafting needs to do a check to make sure the entities are on the person or on the floor, and not in any other storage containers like other people.


Button to jump to first unread

I have joined the 21st century and now have a phone that can access wifi when I'm at home! The little green arrow for clicking to the next unread thing is pretty small and it usually takes me a few tries to hit it. That's okay, but if the first page of a thread had a text link like "jump to first unread" that would make it easier.

-Shoal

"Next Post" "Previous Post" bar?
Maybe an italic link in/near the pagination bar?

Half-fixed by #190 (except not so much on mobile since they can't just Ctrl+F so easily).


Allow the shard store to import ledgers to the local node store

High Level Overview of Change
Enable successful shard imports from the local NodeStore.
Unit tests are pending and will be included in this PR. The progress display requires the node to be run in silent mode.

Context of Change

Type of Change

[x ] Bug fix (non-breaking change which fixes an issue)
[x ] Refactor (non-breaking change that only restructures code)
[x ] Tests (You added tests for code that already exists, or your new feature included in this PR)

Force pushed after rebasing onto develop

Force pushed after rebasing onto develop

Just a quick note that, as it stands, the branch is not building non-unity for me.  ProgressDisplay.cpp is missing #include <sstream>.

Just a quick note that, as it stands, the branch is not building non-unity for me.  ProgressDisplay.cpp is missing #include <sstream>.

I tried to run code coverage locally (using macOS) on this branch.  The run hangs just at the point where the tests have completed.  I expect to see "Checking destination invariants..." in the terminal window but that never shows up, at least after waiting 15 minutes.  So I never got to see the code coverage results.  However if I back out the last two commits, so "Enforce silent mode when running nodetoshard" (efde322fdf887bdd9112f07547fd963dc98666df) is the topmost commit, then codecov completes.
I'm not sure what's wrong, but it looks to me like the "Advance unit tests" (c5d0f3c6ba282aa00c7810e051010362db0f1300) commit is interacting badly with codecov, at least on macOS.
I think that's gonna need to be fixed before this branch can be merged.

I tried to run code coverage locally (using macOS) on this branch.  The run hangs just at the point where the tests have completed.  I expect to see "Checking destination invariants..." in the terminal window but that never shows up, at least after waiting 15 minutes.  So I never got to see the code coverage results.  However if I back out the last two commits, so "Enforce silent mode when running nodetoshard" (efde322fdf887bdd9112f07547fd963dc98666df) is the topmost commit, then codecov completes.
I'm not sure what's wrong, but it looks to me like the "Advance unit tests" (c5d0f3c6ba282aa00c7810e051010362db0f1300) commit is interacting badly with codecov, at least on macOS.
I think that's gonna need to be fixed before this branch can be merged.

The recent commits feature a rework that allows the import process to run during normal server operation (rather than blocking during startup) and does away with the ProgressDisplay in favor of an RPC command (nodetoshard_status) that polls the status of the import.

@mDuo13 this change introduces a new rpc command: nodetoshard_status

I did a very quick scan over the most recent code.  Before diving in deeper I'm curious about something.  @miguelportilla said

A full history server would not be able to start for months as this works now. It's a good job for the Taskqueue.

So this import is now something that happens while the server is running?  And the import can take months?  I'd like some background on expected behavior so I know what to look for...

Am I correct in understanding that now the import takes place while the server is running?  Does that mean the locks that were unneeded during the last review are now important?

What is the expected behavior if the user attempts to shutdown the server gracefully while the import is running and has not finished?

If the import is somehow interrupted before completion and is restarted does the code recover?  Or does it need human intervention to clean up messes?

If the import is interrupted by shutting down the server and then restarted, does the import need to start again from the beginning?  Or can it take advantage of some of the progress made during the interrupted import?

I think answers to those questions will help me be properly oriented while reviewing the code.  Thanks for the help.

I did a very quick scan over the most recent code. Before diving in deeper I'm curious about something. @miguelportilla said

A full history server would not be able to start for months as this works now. It's a good job for the Taskqueue.

So this import is now something that happens while the server is running? And the import can take months? I'd like some background on expected behavior so I know what to look for...
* Am I correct in understanding that now the import takes place while the server is running?  Does that mean the `lock`s that were unneeded during the last review are now important?

* What is the expected behavior if the user attempts to shutdown the server gracefully while the import is running and has not finished?

* If the import is somehow interrupted before completion and is restarted does the code recover?  Or does it need human intervention to clean up messes?

* If the import is interrupted by shutting down the server and then restarted, does the import need to start again from the beginning?  Or can it take advantage of some of the progress made during the interrupted import?

I think answers to those questions will help me be properly oriented while reviewing the code. Thanks for the help.

Yes the import occurs while the server is running and the lock was re-added.
The import process will stop its progress and exit.
No human intervention is needed
The progress is saved and resumed.

Some good news.  The most recent version of your branch lets a local run of codecov on my Mac finish.  I wonder if that's because you're now careful to not leave any threads running after main exits?  But I don't know that for sure.  At any rate, that problem now seems to be fixed.

I just ran local codecov on your branch.  According to codecov the DatabaseShardImp::doImportDatabase() method (the place most of your changes occur) is untouched by any of the standard unit tests.  Are there any manual unit tests that exercise your new code?

I just ran local codecov on your branch. According to codecov the DatabaseShardImp::doImportDatabase() method (the place most of your changes occur) is untouched by any of the standard unit tests. Are there any manual unit tests that exercise your new code?

DatabaseShardImp::doImportDatabase() is invoked by Database_test and DatabaseShard_test (via DatabaseShardImp::importDatabase()).

Nice!  Yeah, I just ran codecov locally using the DatabaseShard unit test.  It looks like the happy path through doImportDatabase() is exercised three times by that unit test.
This is not a requirement, but I encourage you to look for ways to exercise the less-than-happy paths using unit tests.  Some of those corner cases may be fairly easy to get to.  It's not worth killing yourself to get to every line.  But my general feeling is that any code that is not tested probably doesn't work :wink:.

FWIW, the Travis failure with Windows vc2019 is understood and not your problem.
The most recent version of vc2019 added a [[nodiscard]] that causes a warning to be generated in boost code.  Since we compile with warnings-as-errors the build then fails.  Our windows using folks are looking for a fix/workaround.


Tasks with CustomExecution fails if the default terminal profile is contributed by the same extension

Does this issue occur when all extensions are disabled?: Yes

VS Code Version: 1.85.0
OS Version: Any

Steps to Reproduce:

Create an extension that contributes a terminal profile:"terminal": {
    "profiles": [
        {
            "id": "extension.terminal",
            "title": "Custom profile",
        }
    ]
}

From that extension's activation function, launch a task that creates a Pseudoterminal from a CustomExecution:
 const writer = new vscode.EventEmitter<string>();
 vscode.tasks.executeTask({
     definition: {
         type: 'custom',
     },
     name: 'Test task',
     isBackground: false,
     presentationOptions: {},
     runOptions: {},
     source: 'My extension',
     problemMatchers: [],
     scope: vscode.TaskScope.Workspace,
     execution: new vscode.CustomExecution(async () => ({
         onDidWrite: writer.event,
         open: () => writer.fire('open'),
         close: () => {
             throw new Error('Method not implemented.');
         },
     })),
 });

Start debugging. The terminal launches as expected, and nothing else happens.
Change the default terminal profile to our contributed Custom profile.
Restart debugging. VS Code now errors out, complaining that there's no contributed profile for our Custom profile.

If we implement a terminal profile provider, VS Code will start a new terminal with the custom terminal profile instead of erroring out, but I expect the custom task to run.

Any update? This is a serious issue, our extension has a lots of essential tasks wrapped in custom execution and all of them will fail when our users set their default terminal profile.

Bump

Anyone?

This is a serious issue, anyone care to pick it up?

Come on!

This is still an issue @meganrogge any updates on an ETA to fix this?

No updates unfortunately. This is one of many, many issues I am responsible for. Thanks for your patience.

This is still a very annoying issue, more and more users are effected by it. @meganrogge any ETA?

Any update @meganrogge ?

No ETA, this is one of many, many issues I have


Warn when pushing an action to a torn down Microcosm

This PR makes it so that we warn when pushing to a torn down microcosm. This should only happen if an event subscription hasn't been properly torn down when a Presenter unmounts.

Codecov Report

Merging #394 into test-minified-build will increase coverage by <.01%.
The diff coverage is 100%.

@@                   Coverage Diff                   @@
##           test-minified-build     #394      +/-   ##
=======================================================
+ Coverage                99.77%   99.77%   +<.01%
=======================================================
  Files                       25       25
  Lines                      891      894       +3
=======================================================
+ Hits                       889      892       +3
  Misses                       2        2

Impacted Files
Coverage Δ

src/microcosm.js
100% <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 69bbd78...086f276. Read the comment docs.


Removed React favicon

Simple pull request test, deleting unused favicon

Did a lot of clean up of errors, warnings, accessibility, etc.

Rewrote events pages. Uses Meetup API now. Also implemented a button to hide the normal biweekly meetings so people could filter through to the more "special" events like Michael and Daniel's. Some clean up of old files and reimplementation/new implementation of loading state needed.

My f63king man. Looks great.
One note - the pull request comment is currently "Removed Favicon", and you did wahahaaay more than that, so I've renamed it to "Major website cleanup and Meetup,com API integration."
 You normally do this at the end when your stuff has been reviewed and approved.

I'll definitely have to dig into those particulars. I was wonder, as I went along, if there was a better way to handle how I was going about it.
I made that first Pull Request, when I removed the favicon, just as a test. My very first PR. It continued to add any further commits/changes on to that original one. Was there something I should have done to make separate pull requests or can you only have one open at a time for a project?

To make separate pull requests, you have to put the commits on separate branches, and initiate the request from each of those branches. The repo owner @nspilman can then merge them into the repo in a single commit or rebase them for you.

Ah ha -- that makes sense. I was working off of feature branches, which I merged into a dev branch. Once I was sure all of that was working, I would merge dev to master and make the pull request with that.
Noted for next time!

@phoenixcoder Does all of this generally sound correct:
Fork original JDSB Project to your own GitHub
Clone your personal fork locally
Your fork is origin
Set original JDSB Project as upstream
(Potentially fetch from upstream if someone makes changes pertinent to what you are working on? Are there automatic notifications for this, or do you have to check manually? Periodically? Before PR?)
Gaston has been doing some work on the site as well and we were talking about the proper workflow on Monday.

That sounds generally right, if you just want one PR. If someone pushes changes before you push yours, you'll have to

Pull the changes into your local first.
If there's a conflict, perform a manual merge.
Test changes with the new commit(s).
Submit a new PR.

The workflow is generally: whoever submits working changes first, wins! Then either the submitter or the merger/repo owner will perform the conflict merges, if any.

One more for good measure before I let you get back to your day!
Granted, I made my first PR as a test using master branch, so all my subsequent changes to master were added on to this original PR. You've already explained that I should have made PRs from separate branches which is good to know.
What about canceling/closing a PR if you realize you've done something like I did, or perhaps realized a bug of some kind after you've already made the PR. Is that a relatively painless process of simply closing the PR from this page before it has been accepted by the repo owner, or are there unforeseen consequences?
Covering some seemingly simple bases because all of this will probably make it into some kind of "How to contribute" guide, and assuming minimal knowledge from the user is best if you don't know your audience.  How do I fix it?" is even asked goes a long way.

Yeah, you can simply close the PR and make a new pull request. Github tracks the pull request based on the hashcode of the commit. So, if you:

Update the commit, but the hashcode does not change. The subsequent PR will just update your old PR.
Create a brand new commit. The PR will be brand new.
You can also delete the branch your PR was on, and go to a new branch entirely, after you've closed your PR.

You da best! Thanks, chief.


translated to Japanese

I translated documents to Japanese.

ありがとうございます!

Wow. Thank you!

@komi1230 your changes are live! https://yew.rs/docs/ja/intro/