Fix authentication related regression in EC2 driver introduced in v3.3.0

This pull request fixes EC2 driver related authentication regression / bug which was inadvertently introduced in v3.3.0, originally reported in #1545 .
Background, Context
#1519 introduced a bug which would cause EC2 driver to always use signature version 2 for authentication.
This won't work with some newer regions which require signature version 4.
This pull request fixes that and also adds a change which allows user to explicitly specify / override which signature version to use for authentication by passing signature_version argument to the driver constructor.
Thanks
Thanks to @olegrtecno for reporting this issue.
TODO

[x] Changelog entry
[x] Add a release note entry on potential workaround

Codecov Report

Merging #1546 (ba81fa7) into trunk (4370a70) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##            trunk    #1546   +/-   ##
=======================================
  Coverage   83.07%   83.08%
=======================================
  Files         394      394
  Lines       84738    84757   +19
  Branches     9002     9003    +1
=======================================
+ Hits        70399    70418   +19
  Misses      11274    11274
  Partials     3065     3065

Impacted Files
Coverage Δ

libcloud/compute/constants/ec2_instance_types.py
100.00% <ø> (ø)

...d/compute/constants/ec2_region_details_complete.py
100.00% <ø> (ø)

...ud/compute/constants/ec2_region_details_partial.py
100.00% <ø> (ø)

libcloud/compute/drivers/ec2.py
75.45% <100.00%> (+0.03%)
:arrow_up:

libcloud/test/compute/test_ec2.py
97.87% <100.00%> (+0.02%)
: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 4370a70...ba81fa7. Read the comment docs.

Codecov Report

Merging #1546 (ba81fa7) into trunk (4370a70) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##            trunk    #1546   +/-   ##
=======================================
  Coverage   83.07%   83.08%
=======================================
  Files         394      394
  Lines       84738    84757   +19
  Branches     9002     9003    +1
=======================================
+ Hits        70399    70418   +19
  Misses      11274    11274
  Partials     3065     3065

Impacted Files
Coverage Δ

libcloud/compute/constants/ec2_instance_types.py
100.00% <ø> (ø)

...d/compute/constants/ec2_region_details_complete.py
100.00% <ø> (ø)

...ud/compute/constants/ec2_region_details_partial.py
100.00% <ø> (ø)

libcloud/compute/drivers/ec2.py
75.45% <100.00%> (+0.03%)
:arrow_up:

libcloud/test/compute/test_ec2.py
97.87% <100.00%> (+0.02%)
: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 4370a70...ba81fa7. Read the comment docs.

I'll go ahead and merge this since I hope to start voting thread for a bug fix release today.

I'll go ahead and merge this since I hope to start voting thread for a bug fix release today.


Logger / Asyncio Can't Find Running Event Loop

Background
Hello! I'm working to implement plombery based on a request I have to run and monitor a python script on a schedule. This particular script is designed to start at 7AM UTC and run for 23 hours. I'm deploying this app to an Azure Web App service.
Because I am importing another python module that actually holds the logic for the task, I've had to get a bit creative to integrate logging with plombery. Simply passing the logger object to the function didn't work well, so I wrote a function to get the name of the logger just after I call get_logger() from plombery. I then pass the name of the logger to the task and have it use logging.getLogger() to retrieve the logger. Unfortunately, it seems this is still resulting in some bugs related to threading. I've included additional information below, please let me know if you need anything else!
Issue
During the task running, I get MOST of my logs coming through correctly, but also have a significant amount of logging errors showing up in the standard output. A full example of one here:
2023-10-11T15:26:51.337065402Z --- Logging error ---
2023-10-11T15:26:51.367307824Z Traceback (most recent call last):
2023-10-11T15:26:51.367567726Z   File "/opt/python/3.11.4/lib/python3.11/logging/handlers.py", line 1498, in emit
2023-10-11T15:26:51.367598426Z     self.enqueue(self.prepare(record))
2023-10-11T15:26:51.367606226Z   File "/home/site/wwwroot/antenv/lib/python3.11/site-packages/plombery/logger/web_socket_handler.py", line 13, in enqueue
2023-10-11T15:26:51.367612026Z     manager.emit(f"logs.{self.run_id}", record.message)
2023-10-11T15:26:51.367629126Z   File "/home/site/wwwroot/antenv/lib/python3.11/site-packages/plombery/websocket/__init__.py", line 58, in emit
2023-10-11T15:26:51.367634926Z     run_all_coroutines(all_coroutines)
2023-10-11T15:26:51.367640426Z   File "/home/site/wwwroot/antenv/lib/python3.11/site-packages/plombery/utils.py", line 13, in run_all_coroutines
2023-10-11T15:26:51.367645826Z     task = asyncio.create_task(coroutine)
2023-10-11T15:26:51.367651026Z            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2023-10-11T15:26:51.367656126Z   File "/opt/python/3.11.4/lib/python3.11/asyncio/tasks.py", line 371, in create_task
2023-10-11T15:26:51.367661226Z     loop = events.get_running_loop()
2023-10-11T15:26:51.367666426Z            ^^^^^^^^^^^^^^^^^^^^^^^^^
2023-10-11T15:26:51.367671526Z RuntimeError: no running event loop
2023-10-11T15:26:51.367676626Z Call stack:
2023-10-11T15:26:51.428225869Z   File "/opt/python/3.11.4/lib/python3.11/threading.py", line 995, in _bootstrap
2023-10-11T15:26:51.428274870Z     self._bootstrap_inner()
2023-10-11T15:26:51.428282670Z   File "/opt/python/3.11.4/lib/python3.11/threading.py", line 1038, in _bootstrap_inner
2023-10-11T15:26:51.428288270Z     self.run()
2023-10-11T15:26:51.428293270Z   File "/opt/python/3.11.4/lib/python3.11/threading.py", line 975, in run
2023-10-11T15:26:51.428298970Z     self._target(*self._args, **self._kwargs)
2023-10-11T15:26:51.428304470Z   File "/opt/python/3.11.4/lib/python3.11/concurrent/futures/thread.py", line 83, in _worker
2023-10-11T15:26:51.428309770Z     work_item.run()
2023-10-11T15:26:51.428314570Z   File "/opt/python/3.11.4/lib/python3.11/concurrent/futures/thread.py", line 58, in run
2023-10-11T15:26:51.428319670Z     result = self.fn(*self.args, **self.kwargs)
2023-10-11T15:26:51.428324770Z   File "/home/site/wwwroot/app.py", line 21, in gtJob
2023-10-11T15:26:51.428359170Z     gt.runSimulation('PUBLISH', loggerName)
2023-10-11T15:26:51.428364870Z   File "/home/site/wwwroot/models/geo/geo.py", line 2442, in runSimulation
2023-10-11T15:26:51.428370270Z     fetch()
2023-10-11T15:26:51.428375770Z   File "/home/site/wwwroot/models/geo/geo.py", line 2314, in fetch
2023-10-11T15:26:51.428380970Z     plom_logger.info('STARTING TO FETCH AT: ' + str(dt.today().strftime("%m/%d/%Y %H:%M:%S")) + '...')
2023-10-11T15:26:51.428386270Z Message: 'STARTING TO FETCH AT: 10/11/2023 15:26:51...'
2023-10-11T15:26:51.428391271Z Arguments: ()
2023-10-11T15:26:51.431878996Z /opt/python/3.11.4/lib/python3.11/logging/handlers.py:1500: RuntimeWarning: coroutine 'WebSocket.send_json' was never awaited
2023-10-11T15:26:51.431901796Z   self.handleError(record)
2023-10-11T15:26:51.431908696Z RuntimeWarning: Enable tracemalloc to get the object allocation traceback

Additional Issue - Maybe Related?
I saw this before updating to the latest version, but it seems it has returned and might be related. Looks to me like it's the error I'd expect if I was trying to get the logger outside of the task run, which I am not.

Side Note: the dark mode log viewer here has a light font color on a light background, so highlighting is necessary to read. Love the addition of dark mode though!
Code
from apscheduler.triggers.cron import CronTrigger

from plombery import task, get_logger, Trigger, register_pipeline, get_app

import models.geo.geo as gt
import logging

def getLoggerName(taskName):
    loggers = list(logging.root.manager.loggerDict.keys())
    matching = sorted([l for l in loggers if str(taskName) in l])
    return matching[-1]

@task
def gtJob():
    logger = get_logger()
    loggerName = getLoggerName("gtJob")

    logger.debug('Starting GT Job')
    gt.runSimulation('PUBLISH', loggerName)
    logger.debug('GT Job Finished')

register_pipeline(
    id="GeoTab",
    description="Run GeoTab Simulation",
    tasks=[gtJob],
    triggers=[
        Trigger(
            id="daily-seven",
            name="Daily7AM",
            description="Run the pipeline every day at 7AM UTC",
            schedule=CronTrigger(hour=7),
        ),
    ],
)

app = get_app();

if __name__ == "__main__":
    import uvicorn
    uvicorn.run('plombery:get_app', reload=True, factory=True)

Web UI Screenshots

Update: I wrote this without realizing that v0.4.1 had been released. I'm working on updating and deploying that version to see if any of the above has been resolved.

hey, thanks for reporting these issues.
About the traceback styling in dark mode and the wrong "no pipelines" message, I fixed them in v0.4.1
About the logging, there were several bugs, especially when using sync tasks (that non-async functions) and I fixed those in 0.4.0 so if your were using v0.3.* that makes sense you faced those issues. Hope you won't get other bugs on the logging 🥶
So if I get correctly, your end goal is to capture logs in plombery that are not generated with the plombery logger... and yes that is a limitation at the moment but I'd like to find a solution as it's a pretty common situation I think, I'll look into it!

@lucafaggianelli thanks for getting back to me on this!
I can confirm the traceback info box is great now, and the no pipelines message on the front page is gone!
I was using 0.4.0 in the initial post (hence the dark mode) and think the issues I listed are still present in 0.4.1 based on the testing I've done.
The function I'm running is synchronous, so perhaps that does play into the problems I'm experiencing here. I may mess around with running it async, but I cannot make too many changes to the actual process that's being ran here.
The author of the code I'm running included logging, but only using print statements. I think it would be cool if plombery captured any stdout within a task and piped that to the logger somehow.
In the meantime, I've just gone through and added a logger.info call wherever he had print statements, although some modification had to be done since print works differently. The tricky part was getting the logger passed down to his module.
logging.getLogger() was the solution, but in order to get the correct logger I needed the name. Currently I'm just looking at all of the loggers in the Logger dictionary and getting the newest one with the name of my task. Then I pass that name down and call getLogger(plomLoggerName) inside of his module so I can use it.
I think adding a getName function on your custom logging instance would be nice and make that a bit easier.
I'm interested to hear your thoughts around the async / sync stuff since you've been trying to address that. I'm gonna mess around with it on my end a bit and see what happens. If nothing helps then I'm likely going to be opening up another issue momentarily as I'm having issues with this job not running properly on the schedule, but a manual run seems to work fine.

Based on this line in the error: /opt/python/3.11.4/lib/python3.11/logging/handlers.py:1500: RuntimeWarning: coroutine 'WebSocket.send_json' was never awaited and the traceback, I did some looking around in the plombery source code.
You're probably aware of this, given that you built it, but it seems this is related to this old issue on the FastAPI side: https://github.com/tiangolo/fastapi/issues/98
Based on the code in the files below and reading through that issue (and others) I think you've done this correctly, just not sure why issues are still occurring - other than it's probably something to do with the fact that I'm trying to run synchronous code, which might be blocking some of this from running periodically.
I mostly wanted to post this comment as a summary of what I think is happening, but also to provide some context on my deployment. I am developing on Windows but deploying to a Linux Web App on Azure.
Startup Command: gunicorn -w 4 -k uvicorn.workers.UvicornWorker  app:app
Folder structure:

https://github.com/lucafaggianelli/plombery/blob/88e72a6f74352caeeb1446cae856e7a2f0abf3c7/src/plombery/api/__init__.py#L1-L21
https://github.com/lucafaggianelli/plombery/blob/88e72a6f74352caeeb1446cae856e7a2f0abf3c7/src/plombery/websocket/__init__.py#L75-L88
https://github.com/lucafaggianelli/plombery/blob/88e72a6f74352caeeb1446cae856e7a2f0abf3c7/src/plombery/api/routers/websocket.py#L1-L16

Hey thanks for the detailed debug, I'll take a look asap.

Update: the creator of the script I'm running made some changes to fix some bugs in his logic, and I altered my startup command to fix another issue that I opened and closed here, and it seems that as a result of those actions, this issue is also gone. I am no longer seeing these messages in the logs.
New startup command: gunicorn --threads 4 -k uvicorn.workers.UvicornWorker app:app

Did you try to run uvicorn directly, without passing from gunicorn? I'm wondering if the WSGI nature of gunicorn, even though you're using the UvicornWorker, may lead to your issues.
Anyway happy that you solved the problem! In general, sync tasks should work in plombery, but apparently there is still something which I didn't test properly, I'll try to improve the sync testing!
PS: I'm closing this issue as you solved it, though if you need, you can reopen it

I didn't mess around with using uvicorn in production mostly because the Azure Web App service documentation on this was centered on gunicorn and I'm not really familiar with either of them personally.
That said, I did use uvicorn while testing locally and experienced the same issues, it just seemed to be less frequent. And none of this was happening before I implemented the logging stuff, so I think perhaps there's something weird with sync tasks and logging? Who knows. If anyone else experiences the same thing, feel free to @ me and we can compare and see if we can find the disconnect.
All good for now though, thank you!

@lucafaggianelli I'm getting the same issue since I started using get_logger():
Traceback (most recent call last):
  File<EMAIL_ADDRESS>line 1498, in emit
    self.enqueue(self.prepare(record))
  File "/Users/morpheus/Desktop/bazarr_provider/venv/lib/python3.11/site-packages/plombery/logger/web_socket_handler.py", line 13, in enqueue
    manager.emit(f"logs.{self.run_id}", record.message)
  File "/Users/morpheus/Desktop/bazarr_provider/venv/lib/python3.11/site-packages/plombery/websocket/__init__.py", line 58, in emit
    run_all_coroutines(all_coroutines)
  File "/Users/morpheus/Desktop/bazarr_provider/venv/lib/python3.11/site-packages/plombery/utils.py", line 13, in run_all_coroutines
    task = asyncio.create_task(coroutine)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File<EMAIL_ADDRESS>line 381, in create_task
    loop = events.get_running_loop()
           ^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: no running event loop

Anything I should look for?
Thanks by the way for that great piece of software you made!


Remove deprecated XML tag support

Warnings

:warning:
It looks like this PR contains functional changes without a corresponding test.

  Generated by :no_entry_sign: dangerJS against 520e872fae217cf49314334661e11dabfd740ab7


testfixture: Make recipe-function callbacks typesafe without breaking compatibility and rename callback to func

@kwk snapshot fabric8-wit image is available for testing.  docker pull docker.io/fabric8/fabric8-wit:SNAPSHOT-PR-1620-1

Codecov Report

Merging #1620 into master will increase coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1620      +/-   ##
==========================================
+ Coverage    53.8%   53.91%   +0.11%
==========================================
  Files         127      127
  Lines       16289    16289
==========================================
+ Hits         8764     8783      +19
+ Misses       6841     6823      -18
+ Partials      684      683       -1

Impacted Files
Coverage Δ

remoteworkitem/scheduler.go
63.26% <0%> (+16.32%)
:arrow_up:

remoteworkitem/github.go
100% <0%> (+21.62%)
:arrow_up:

models/transaction.go
63.63% <0%> (+27.27%)
: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 8050410...fb5cb31. Read the comment docs.

I was facing these issues after master pull
# github.com/fabric8-services/fabric8-wit/test/testfixture
test/testfixture/make_functions.go:456: fxt.runCustomizeEntityCallbacks undefined (type *TestFixture has no field or method runCustomizeEntityCallbacks)
test/testfixture/recipe_funcs.go:421: undefined: CustomizeEntityCallback
test/testfixture/recipe_funcs.go:423: fxt.checkCallbacks undefined (type *TestFixture has no field or method checkCallbacks)
FAIL	github.com/fabric8-services/fabric8-wit/controller [build failed]

I did fix that but wondering how jenkins passed!


Align FORCE_COLOR=0 with Node.js meaning

Node.js allows disabling color by passing FORCE_COLOR=0 (https://nodejs.org/api/tty.html#writestreamgetcolordepthenv). Would you be open to a PR checking if FORCE_COLOR is 0?
For context, we just migrated Babel from Chalk to picocolors and found this difference. For now we are going to roll our own color detection logic to preserve the Node.js-like behavior.

I'm considering switch to https://nodejs.org/api/tty.html#writestreamhascolorscount-env that seems to be available since Node.js v10.16. I will try to find some tests to see how it works with FORCE_COLOR variable and will make sure too keep this behavior in mind. There's probably a small patch I can make in the meantime for v1 to avoid breaking changes.

For what is worth, I tried running a totally unscientific poll on Twitter and it seems like people are split on the behavior of FORCE_COLOR=0. While I still think that it's good for Node.js libraries to align with Node.js, changing what FORCE_COLOR=0 does should probably be considered a breaking change.

because NO_COLOR= node ... will trigger that condition, but still be a "" value.

Good point, that's something worth fixing. Not quite sure how one would end up in situation like NO_COLOR= node 😅 .

Not quite sure how one would end up in situation like NO_COLOR= node

Easy. That would be one of the thing I would try to override NO_COLOR that was set somewhere before this line.
For example
export NO_COLOR=1
node aaa
node bbb
# I want only this command to use colors (and my expectation failed)
NO_COLOR= node ccc
node ddd
...

Alright, I finally got a moment to fix this: https://github.com/alexeyraspopov/picocolors/pull/87. Should also cover the issue with dynamic require() calls once and for all.

Version 1.1.1 is published.


TP- Link switch not working after 0.34

Make sure you are running the latest version of Home Assistant before reporting an issue.
You should only file an issue if you found a bug. Feature and enhancement requests should go in the Feature Requests section of our community forum:
Home Assistant release (hass --version): 0.34.0
Python release (python3 --version): 3.4.2
Component/platform: switch. tplink
Description of problem:
After the ugrade to 0.34, the tplink hs110 stop working as intended.
they show up on the HA ui, but the button makes no action. also, all the data regarding the status of it does not work . e.g, power consuption, daily consuption, status)
already removed the switch and added it again, it´s found but with the same issue.
no issues displayed on the log file.
Expected:
Problem-relevant configuration.yaml entries and steps to reproduce:

Traceback (if applicable):

Additional info:

Could be that #4670 is causing trouble. @GadgetReactor perhaps you can help here.

Yup, the identify_model function is broken in that version.
https://github.com/GadgetReactor/pyHS100/blob/53c06ca7844f16d6ffe068f4ca01e8d5c707b9e3/pyHS100/pyHS100.py#L256
when you strip HS from HS110(EU) you still don't have an int, and it matches everywhere for against an int. I'd recommend reverting that dependency upgrade.

thanks @mwinelt
can you share some lights on how to do that?
what specific dependency are we talking about?

@pereiraru Check https://github.com/home-assistant/home-assistant/pull/4722


Teach exclusivity access marker verifier to handle UnsafePointer.

My previous attempt to strengthen this verification ignored the fact that a
projection or addressor that returns UnsafePointer can have nested access after
being passed as an @inout argument. After inlining, this caused the verifier to
assert.
Instead, handle access to an UnsafePointer as a valid but unidentified access. I was trying
to avoid this because an UnsafePointer could refer to a global or class
property, which we can never consider "Unidentified". However, this is
reasonably safe because it can only result from a nested access. If a global
or class property is being addressed, it must already have its own dynamic
access within the addressor, and that access will be exposed to the
optimizer. If a non-public KeyPath is being addressed, a keypath instruction
must already exist somewhere in the module which is exposed to the optimizer.
Fixes rdar://problem/41660554 Swift CI (macOS release master, OSS): SIL verification failed: Unknown formal access pattern: storage.

@swift-ci test.

Build failed
Swift Test OS X Platform
Git Sha - b5fd073166f545295ae68798de9699280a0cecfd

Build failed
Swift Test Linux Platform
Git Sha - b5fd073166f545295ae68798de9699280a0cecfd

@swift-ci test.

Build failed
Swift Test OS X Platform
Git Sha - ba3b6ee666eeb6b0c5c71ab302d59acd623ed31a

Build failed
Swift Test Linux Platform
Git Sha - ba3b6ee666eeb6b0c5c71ab302d59acd623ed31a

LGTM

@swift-ci smoke test and merge.

@swift-ci smoke test and merge.

@swift-ci smoke test and merge.

@swift-ci smoke test and merge.

@swift-ci smoke test.


Issue with k8s.io/docs/setup/independent/create-cluster-kubeadm/

This is an initial problem

[ ] Feature Request
[ X] Bug Report

Problem:
By using own eyes and ordinary reading & understanding I am not capable of using proxy. Everytime I start proxy and doing http/:localhost:8001 I am getting:
http: proxy error: x509: certificate signed by unknown authority (possibly because of "crypto/rsa: verification error" while trying to verify candidate authority certificate "kubernetes")
is there any chance that you make your documentation understandable and workable?
Btw, should proxy be started on... which machine? you assume that we don't have any idea how are you organize your... thing. Please, help.
Proposed Solution:
Page to Update:
http://kubernetes.io/...

Hi @vglisin
Are you able to communicate with your cluster using kubectl cluster-info without any errors? I'm not anywhere near an expert on the crypto pieces, but I've used kubectl proxy many times without that issue, so I'd like to try and and help.
The kubernetes cluster is generally running all of it's containers and services on a private, internal and isolated network, with only the API server exposed, and that with some of this crypto in your kubectl context and configuration to access it. When you use the command kubectl proxy, it's making a proxy connection on your local machine (where ever you're running the command from) to your Kubernetes cluster through the API server, so that you can access that API server through your localhost on port 8001 without all the overlay crypto authentication in place. kubectl proxy (and kubectl port-forward) are two commands to basically make it more convenient to work with the deployments you've created and access the Kubernetes API directly, should you want to.
Were you following a specific tutorial set, or set of setup instructions, when you ran into this issue?


MWPW-162832- Pass locale code to prelude

Pass locale code for locale mapping at prelude side

Resolves: MWPW-162832
Test URLs:
Before: https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=stage
After: https://main--cc--adobecom.hlx.page/drafts/ruchika/remove-background1?unitylibs=locale-code

Hi @Ruchika4 In place of splitting the url to find the locale and parsing through the object could we use the getConfig().locale? This property returned by config has the correct entry of the locale from the current url and can return the required details.

Code has been updated. Please check.


Refactor into monorepo structure

Making this PR now so that we can discuss how to structure the potential monorepo. I've already moved some things around, but this probably broke several things that then need to be fixed. Please write any relevant thoughts you have.

A monorepo is exactly having several different but related projects in one repo. If you just have one big thing in the root, that's a monolith and exactly what this PR is about moving away from. If you think we should stick with a monolith, that's a fair position.
Fundamentally, the training code does not care for nor need the dependencies from the preprocessing code. When I clone this repo on LUMI, I have no reason to install from the pyproject.toml in the root because the setup is completely different and relates to training. Same goes for evaluation. Nonetheless, there are some dependencies between these things, which is the purpose of putting them in the same repo.

Can you describe the setup and your thoughts behind it?
I'm for instance a bit confused about how the llm-foundry fork interacts with the repo post-PR. Is it an installed module? Is it part of the code base?

Agreed on this 👍

Having separate config files floating around seems a bit silly - in that case we're not really mono repo'ing, and we could just as well have two separate repos?

Just a nitpick, but having different pyproject.toml-files does not make it less of a monorepo. It means we have multiple packages in the same repo, but that's exactly what we're looking for 👍
@rlrs The motivation here was that separate parts of DFM needed the same dependency, but with different versions, right? We don't need to have separate release pipelines or anything?
If that is the case, I think @saattrupdan is right, that the easiest transition is to have a pyproject.toml with different dependency groups. As long as the external APIs of the dependencies is stable (perhaps a big if?), this shouldn't cause problems with type-checking.
We could also go separate packages.

llm-foundry was and still is a submodule. I've put it as a submodule because I not only want to install it using pip, but I want to develop on it and perhaps upstream changes using git. That's what a git submodule is for.

llm-foundry was and still is a submodule. I've put it as a submodule because I not only want to install it using pip, but I want to develop on it and perhaps upstream changes using git. That's what a git submodule is for.

No reason to get snappy here, I just didn't know what a git submodule was in that case, but thanks for the explanation!
I found this explanation of a submodule:

It often happens that while working on one project, you need to use another project from within it. Perhaps it’s a library that a third party developed or that you’re developing separately and using in multiple parent projects. A common issue arises in these scenarios: you want to be able to treat the two projects as separate yet still be able to use one from within the other.
Here’s an example. Suppose you’re developing a website and creating Atom feeds.  You’re likely to have to either include this code from a shared library like a CPAN install or Ruby gem, or copy the source code into your own project tree. The issue with including the library is that it’s difficult to customize the library in any way and often more difficult to deploy it, because you need to make sure every client has that library available. The issue with copying the code into your own project is that any custom changes you make are difficult to merge when upstream changes become available.
Git addresses this issue using submodules. Submodules allow you to keep a Git repository as a subdirectory of another Git repository. This lets you clone another repository into your project and keep your commits separate.

That sounds great.
Also, @rlrs, would keeping the dependencies of the data processing and training separate resolve the repo issue?

Wasn't meant that way, was just intended as a shorthand for what you linked!

It's all good 🙂

Splitting dependencies probably goes a long way here. Training requires a separate virtualenv from preprocessing requires a separate virtualenv from evaluation. As for training (and eval on lumi) it all has to run in a specific container, too. Even the venv has to be installed while in this container.
I think the essence is that I don't believe it to be cleaner to have one src/ directory than to have a functional split based on purpose.

Ah, I think I understand it now - the different environments and especially the need to run in a container for the training bit makes it more complicated than simply having the same core setup and installation method. I think why a monorepo structure would make things easier in that case, when the setup is really quite different on the two different sides.
Will there be any need to import things across the data processing and training, or are they completely separate? And if so, is that possible with the monorepo structure?

We talked about this at the office, and I believe we achieved a common understanding of this monorepo structure making sense, even though the different projects really aren't connected.
We could probably do a separate repo for each project now, but in the future we would like to have a joint pipeline for data -> training -> evaluation.
@HLasse @KennethEnevoldsen any comments from you besides the above?

We talked about this at the office, and I believe we achieved a common understanding of this monorepo structure making sense, even though the different projects really aren't connected.
We could probably do a separate repo for each project now, but in the future we would like to have a joint pipeline for data -> training -> evaluation.

So what is the conclusion from the discussion above -  I am unsure?

Regarding the CI thing, it seems like it's super easy to run separate parts of CI for different subfolders.
See here: https://stackoverflow.com/a/67941131/3154226

Great, do we fix this in another PR? Also there might be some paths that are broken now, and misc. things like the Makefile not making sense.

Great, do we fix this in another PR? Also there might be some paths that are broken now, and misc. things like the Makefile not making sense.

Yep, let's fix these things in other PR(s).


[Enhancement] Evaluate project boards

Description
Organizing tons of issues can be tideous and as a product owner, I want to see new issues and progress at a glance. Evaluating options to present our issues in Kanban style would be great.
Criteria

evaluation done
presented and discussed in this issue
decision made

How about creating a new project with Githubs native project boards for projects (beta) and linking a repository as PoC?
Afterwards basic automation for projects by inserting built-in keywords (To do, In progress, Done) into issues or PR should be provided out-of-the-box. Other columns with keywords can be added by configuring automation for project boards.
Maybe boards also provide or will provide a native possibility for issue time tracking and thresholds looked for in  Enhancement: Stale issues/PRs notifications #4 .

@pdq21 I had a very first glance at the project boards, but it was somewhat unintuitive, since the control was label based and one had to add and remove labels or grant access to the board. If you are interested in evaluating this in details, you can grab the story and I can set up a testing repository for the same.

It seems one has to be owner or admin to create an organization project (beta) and an organization-wide project board, meaning the project and the project board have to be created for writers and readers to access them.
There are three Project board permissions for an organization:

READ view
WRITE view, link, interact
ADMIN above and settings and permission

For starters the Basic Kanban board template with basic automation should suffice.

Update:

The Automated Kanban template needs to be selected for semi-automation.
Other templates include bug triage and review.
Issues from linked repos initially need to be inserted by hand either within the board or the issue.
Afterwards these issues are somewhat automated.
Usefulness has to be evaluated.

Hi @pdq21,
As I stated, I can prepare this stuff, if you want to take over the evaluation. But you can also create a testing organization on your own.
Do you want to take over this issue? It is quite uncritical for me, since we don't have that many tasks for now.

I already tried yesterday and I can set up a PoC within the org. The experience can come in handy even if there are not too many issues/PR right now.
Since the issues have to be imported manually it is only semi-automatic. Therefore I tried the https://github.com/marketplace/actions/send-tweet-action and got it to work after fiddling with Github secrets.
I also tested the fully-fledged app ClickUp with boards and calendars.

I had a quick look at ClickUp over the weekend, and it seems really promising. I would love to get a better idea of it. In case you want to take care, I can enable the organization to use it.

Sure. I would suggest to begin with a simple built-in legacy board and advance to ClickUp if need be.
The new boards (beta) offer more sophisticated workflows but crashed during testing.
If automatic adding of new issues and PRs is intended, a repo-token with perms repo and write:org has to be added for the action to access the repo.
An admin has to create a legacy board and assign the relevant permissions to members.

I will have a look over the weekend :)

At least for now, the GitHub project board seems promising. Nevertheless, there is not enough demand to maintain such a board, for now. -> closing, but can be reopened/recreated if needed.


Creating the Type Plugin: BIOM type

This depends on https://github.com/biocore/qiita/pull/1686 so review/merge that one first.
This adds a new type plugin to the system: the BIOM type plugin.
This plugin defines the type BIOM in the system. Since BIOM is a central type in Qiita, I think it is worth to have it as it's own package, since all the processing plugins will depend on this type but not necessarily in the other target gene types.
Currently, only the "create artifact" functionality is implemented: it checks that the sample ids in the BIOM table match the ones in the prep template and if not, tries to fix them. If it can't the artifact is not added to the system and provides information to the user on how to best solve the issue.
The HTML summary generation functionality is currently stubbed out on how it should work in general, and @antgonza will help to add such functionality.

One comment. :+1: once my comment is addressed and the tests pass. @ElDeveloper @mortonjt could you take a look?

Few light comments, :+1: otherwise

Thanks @josenavas and @wasade


Option to change the 'eks_worker_sg' inbound source IP for ports 30000-32767

Is your feature request related to a problem? Please describe.
This feature request as about an option to change the inbound source IP for the 'eks_worker_sg' Security Groups configured in Helm using the ports 30000-32767 . By not being in control of this source IP a potential security risk is created as discovered by our companies security team.
Describe the solution you'd like
I would like to have the option to change the inbound source IP to either a range or specific IP. This way the Security Group is more secure by limiting the inbound connections.
Describe alternatives you've considered
I tried to change the rules manually within the AWS console. Unfortunately AWS EKS 'detects' these changes and will not remove/updates these rules anymore. This eventually results in the rule limit being reached causing AWS EKS being unable to add new rules.
Additional context
Screenshot of the Security Groups with the rules containing the <IP_ADDRESS>/0 IP we want to change:

Files to change:

shared/charts/ambassador/templates/service.yaml ->add 'loadBalancerSourceRanges'
shared/charts/ambassador/values.yaml -> add variable (for example: 'loadBalancerSourceRanges:')
shared/configuration/roles/setup/ambassador/task/main.yaml -> line 52 -> --set ambassador.loadBalancerSourceRanges={{ network.env.loadBalancerSourceRanges }}

Added the following lines to the ./ambassador/templates/service.yaml:

Added the 'loadBalancerSourceRanges' value to ./ambassador/values.yaml:

Added '--set ambassador.loadBalancerSourceRanges={{ network.env.loadBalancerSourceRanges }}' to 'KUBECONFIG=' in the ./ambassador/tasks/main.yaml on line 52:

Added 'loadBalancerSourceRanges:' to the network-cordav2.yaml:

These changes make it possible to add custom IPs to the inbound rules for the ports in the range: 30000-32767

Reopening


[How to] Connect with fellow developers?

I find this OpenVINO Notebooks repo much useful to demonstrate the practical usage of OpenVINO Toolkit in python. I would like to know is there any platform like slack or discord for the particular repo. Would love to join and contribute...
Thank you :)

Hello @mahimairaja,
Unfortunately we don't have established public channels except posts here. We'll discuss internally regarding this. So far feel free to post questions/suggestions here

Gotcha...


feat: Add build script for firefox add-on

Summary
Add build script for firefox add-on (close #8).

Add scripts/build.firefox.sh (and move build.sh to scripts/build.chrome.sh; build script for Chrome).
Integrate the logic for generating manifest.json for both of the Chrome and Firefox build script."

Chrome is not happy with background.scripts and insists on using background.service_worker
Firefox doesn't support background.service_worker and wants background.scripts

How did you test this change?

@unvalley just shared with you that Firefox support has now been added ;)
There were some differences in the manifest.json fields, so generating it during the build process. If you have a better approach, feel free to send a PR. I will proceed with the submission to the Firefox web store ASAP.

Thank you!!


Selecting library entrypoint over binary entrypoint

I might be missing something, but it appears to me that we're unable to select the library entrypoint if there are any binary entrypoints.
I've tried to rename my binary entrypoint and use data-target-name to select the library, but to no avail. The moment there is a binary entrypoint, whether implicitly via an existing main.rs, or explicitly with a different name declared in Cargo.toml, it seems trunk wants to compile that entrypoint.

I have an issue with that since I want to use embassy-executor - yes, I'll likely have both that async executor and wasm_thread in my webpage in the end :sweat: - , which comes with the #[embassy_executor::main] macro, and I can't make that work on a binary entrypoint on wasm. I might be able to work around that, but frankly I'd prefer not to do it that way.
If I didnt overlook something, I could try and make a PR for that. I'm uncertain about the API you would prefer. I think an explicit data-lib attribute to select the library would make for the easiest discovery and best errors, but alternatively I could also try and fix the selection of the compilation target. However then, from my understanding, people will always have to have specifically named binary entrypoint when they also want to target native as well, because AFAIK both the library and default binary entrypoint take their name from package.name

My need for this is gone after all. Still happy to help, if you want this.


popsicle.plugins.parse(['json', 'urlencoded']) results in EPARSE error on url not found

when i tack on .use(popsicle.plugins.parse(['json', 'urlencoded']))`
to a popsicle request, if the url is not found, it results in a EPARSE error.
If I don't tack on the parsers, it gives the appropriate error/server response...

If the response is a 404 and the body can not be parsed, what is the expectation? This seems to work as expected. If you’d like it to error on a non-2xx status code, there’s a plugin for that - but it doesn’t by default. If there’s some other behaviour you desire, you could also wrap the function in your own behaviour to parse on this or do something else when failed.

the expectation is to get the 404 response, not a parse error, as the plugins are tacked onto every request. The plugins should not break regular functionality

Exactly! They are for every request, even non-2xx. If you want custom handling, it's really simple - just write something that goes one way or the other - parses or doesn't parse. It doesn't come with that built in, but could in the future - the reason for not building it in is that if you're using JSON parse you're expecting a JSON response. You're not getting a JSON response though, so it makes sense to error.
Let's start with what you want to happen here, then I can help find the right solution. Right now I still don't know what you want, sorry. You do a req -> 200 -> parse and, what, req -> 404 -> not parse or req -> 404 -> error? The normal way I'd handle this is with https://www.npmjs.com/package/popsicle-status, since I don't want to handle 4xx or 5xx like the rest of the application.

The "req -> 404 -> not parse" route, but i will look at the status package


FsRtl-functions are not structure members

For example: FsRtlCopyRead had its own separate page - https://msdn.microsoft.com/library/windows/hardware/ff545791(v=vs.85).aspx

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

ID: e4fa42b5-1632-42e6-8d7d-05778b6eeaa3
Version Independent ID: ad17836c-e33f-a306-6912-e5ad12d46c6a
Content: _FSRTL_ADVANCED_FCB_HEADER
Content Source: wdk-ddi-src/content/ntifs/ns-ntifs-_fsrtl_advanced_fcb_header.md
Product: windows-hardware
GitHub Login: @windows-driver-content
Microsoft Alias: windowsdriverdev

Already fixed in source


About this project

Will this ever release? Because there are many developers(you can't imagine how much! Some includes Aliens too...) waiting to see Semantic UI in Angular JS. If so when can we expect the first release?

@m0t0r owns this now, but he's probably busy with work. It would probably speed things up if other contributors started submitting pieces

I am back after Feb, 15. It will be released!

kool!

Any news?

Hi. I am currently alone in this project and taking into account that I am not dedicated full time to this project but rather on free time which is pretty often catastrophically insufficient. It would be just terrific to get any support from the community!

I would like to try to start contribute to the project. Should I start from this repository or the old one (https://github.com/caitp/angular-semantic) ?

You should start from this one. I'm happy to review patches, so don't hesitate to ping me if you've got something =]

Awesome! Many thanks!

Is there a roadmap somewhere ? Trello ? A good idea would be to have a first version covering at least the same features as https://angular-ui.github.io/bootstrap/

there isn't currently, would you be interested in owning that?

I am very interested in collaborating. I've using Semantic + Angular for some months, and I'd have loved to have something like this.

@caitp it is interresting  but I really do not have the ressources yet to do so, at least not at as a leading position. What is Semantic-UI-Angular expecting to build ? A directive based framework or replacing jQuery with angular ? @Darkeye9 how are you using semantic-ui ? There is definitely a lot of interest for the project, + 400 stars if we count the former version one.

@Xample I am writing the templates in HTML and including the Semantic stuff by hand, I ussually add some logic to Semantic using ng-class or ng-if attributes from Angular. For the JS components Im calling the Semantic constructors in the Angular controllers.
Where I have done the most hard work is at integrating the forms validation into Angular, by following some of the advices exposed here. But it needed some tinkering nevertheless.

I like Semantic-ui!! I hope  can contribute to this project.

@ethansure you absolutely can and should contribute. I'm a bit preoccupied with other projects atm, but I'm happy to review code and offer some guidance if needed. It would be great if some volunteers took up a strong roll in driving the project forward

I would like to dig into this. I think there needs to be a discussion about goals, scope, development guidelines, and some of the housekeeping stuff (organization, namespacing, conventions, etc.) because that stuff is harder to change once people start slamming in commits. If nothing else, I think we should borrow heavily from, or at least use as the baseline, ui-bootstrap because they've already established a decent working project.

Anything you guys need to help organize, let me know. Perhaps start with gitter?

I think there needs to be a discussion about goals, scope, development guidelines, and some of the housekeeping stuff (organization, namespacing, conventions, etc.)

Personal opinion, but whoever is able to take more time on this is welcome to disagree:

scope: strong primitives which enable developers to build good looking, functional, accessible applications with ease. This means, provide good fundamentals, and make sure those fundamentals are easy to use, and accessible by default. More interesting stuff built ontop of those fundamentals are allowed also, but start with the fundamentals (form controls, layout/navigation helpers (like modal windows, tab controls, whatever you like), etc).

development guidelines. this is mostly covered above, aim for accessibility, performance, and extensibility. Some more general API guidelines, prefer dictionaries of options to lists of formal parameters (it's easier on everyone). Try to avoid duplicated code as much as possible. Try to keep "hot" internal methods monomorphic, so that VMs can do a better job of optimizing them --- but avoid premature optimizations, always measure. This is basically the same general stuff you'd tell anybody on any software project, so hopefully none of it is too surprising.

housekeeping stuff. There are some things that we probably want here. @jlukic can talk about their CLA strategy, if any --- other devops-y stuff like release schedules, code reviews, ownership, etc, should be discussed on gitter or irc, generally.

organization. I mean, whatever makes sense --- since @m0t0r has already gotten started with this, it's fine to follow his lead. If it gets too crazy in a few months, don't hesitate to re-org, just make sure other contributors are on the same page, so that you don't step on anyones feet.

namespacing. I'm not too keen on "namespacing", but I think @m0t0r has started prefixing things with "sm" (is that right?) --- whatever you want to do is fine, just try to be consistent.

conventions. In terms of code style, my preferences are: linewrap at 100 cols, no single-letter variable names, concise but descriptive method names, 2 space indentation, no space bettwen "function" and "(" or function-name and "(", space before left-braces and all other left-parentheses, single-statement blocks (like if (foo) return bar;) are okay if the single statement is a return statement, otherwise always put them in braces (embrace them!) --- space before and after all binary operators, space before and after conditional ? and :, no space to the left of : for labels or property names... I think most of these rules are covered in some of the default jscs configs, but ymmv. If people have their own preferred styles, can certainly absorb some of it in, just try to keep the project consistent. clang-format is a big help, but might be too much to ask most people to do.

In terms of API conventions, I think a lot of this is covered above. If you're not sure about something in particular, don't hesitate to ask.
I think that covered all of those --- the answer is basically "whatever, but here's what I like", so for sure everyone interested is welcome to chime in and provide their own opinions.

Hi! @caitp made a good overview. What I can add is the project already has .jscsrc with established conventions based on the one from angularjs. Yes, I started prefixing directives with sm, that is correct.
@HackedByChinese mentioned about module naming. Well, particularly about .elements part I can tell that I took it from Semantic-UI itself, as they have a few categories such as: Elements, Collections, Views, etc. So I thought to be closer to the framework way. But we can discuss on this, absolutely.
As an additon, I try to keep [angular material project] (https://github.com/angular/material) as a baseline for this project, with all respect to [ui-bootstrap project] (https://github.com/angular-ui/bootstrap). But everything is open for discussion.

Hi, I like the way semantic ui works and love to use the integrated version with angular. I want to contribute some code in this project if possible.

Hi @mdgs, it's totally possible, and you can totally do it, and we will even review it, check it in, and publish it on npm and bower. By all means, contribute =)

Do we still need this issue being open?

Maybe handy until the information in it is represented in the readme.

Dear everyone,
I think that you should create a CONTRIBUTING.md file and put all of your guideline on it for all new contributors.
Contributing Guidelines

For those that are interested, why not just start with something like this and then build on it?

  $.each($.site.settings.modules, function(index, module){

    var fn = $.fn[module];
    //var options = $.fn[module].settings;

    if(module == 'form' || module == 'api' || module == "state" || module=='visibility'){
      var name = 'se' + _(module).ucfirst();
    }else{
      var name = module;//'se' + _(module).ucfirst();
    }

    app.directive(name, function() {
      return {
        restrict: 'AC',
        //require: 'ngModel',
        seModule: {
          'name': module,
          'fn': fn
          //'options': options
        },
        scope: {
          options: '&',
          ngModel: '='
        },
        controller: ['$scope', '$http', function($scope, $http) {

          //$scope.getTemp = function(city) {}

        }],
        link: function(scope, iElement, iAttrs, ctrl) {

          if(!scope.options){
            scope.options = {};
          }

          scope.options.directive = scope;

          scope.options.onChange = function(value, text){
            scope.ngModel=value;
            if(!scope.$$phase) scope.$apply();
          }

          $(iElement)[module](_.clone(scope.options));

        }
      }
    });

  })

This gives us full access to ALL of the modules in semantic as well as full access to all of the options.  As we develop, we can create more advanced templates, but this is at least a good start.

@AddoSolutions thank you for your contribution, we might think about similar solution later on.
NOTE:
Use of
if(!scope.$$phase) scope.$apply();

is known old anti-pattern. Don't use such things in your code.

Thank you everyone who contributed to this issue but I am closing it as it gets similar to a garbage bin. If you have any questions/problems/suggestions/ideas create a new issue or a PR.

@m0t0r Indeed, that was some ghetto code.  Curious though, how I would get the scope.$apply() higher in the call stack?

I think waiting or writing for angular 2 instead, wont be a waste of effort


[SPARK-29434] [Core] Improve the MapStatuses Serialization Performance

What changes were proposed in this pull request?
Instead of using GZIP for compressing the serialized MapStatuses, ZStd provides better compression rate and faster compression time.
The original approach is serializing and writing data directly into GZIPOutputStream as one step; however, the compression time is faster if a bigger chuck of the data is processed by the codec at once. As a result, in this PR, the serialized data is written into an uncompressed byte array first, and then the data is compressed. For smaller MapStatues, we find it's 2x faster.
Here is the benchmark result.
20k map outputs, and each has 500 blocks

ZStd two steps in this PR: 0.402 ops/ms, 89,066 bytes
ZStd one step as the original approach: 0.370 ops/ms, 89,069 bytes
GZip: 0.092 ops/ms, 217,345 bytes

20k map outputs, and each has 5 blocks

ZStd two steps in this PR: 0.9 ops/ms, 75,449 bytes
ZStd one step as the original approach: 0.38 ops/ms, 75,452 bytes
GZip: 0.21 ops/ms, 160,094 bytes

Why are the changes needed?
Decrease the time for serializing the MapStatuses in large scale job.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing tests.

ping @dongjoon-hyun @holdenk @viirya

Thank you for pinging me, @dbtsai . I'll take a look tomorrow.

I haven't looked at the code yet, can you clarify what ops/ms is measuring here?

For smaller MapStatuses, we find it's 2x faster.

Just to verify, this holds true for small jobs, where you have 10 outputs or 100 outputs?
Did you try with say 2000 or 5000 blocks?  The performance numbers for zstd ops/ms for the 2 step dropped a lot between 5 and 500, whereas the 1 step didn't drop nearly as much, wondering if 1 step becomes faster at some point.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/16916/
Test PASSed.

@tgravescs it's record / ms. When the num of blocks are large, two steps and one step will have similar result, but two step will never be slower.
I switched to use org.apache.commons.io.output.ByteArrayOutputStream instead of the standard ByteArrayOutputStream, and saw another 25% performance gain.
In Apache's ByteArrayOutputStream doc, it says
/* In contrast
 * to the original it doesn't reallocate the whole memory block but allocates
 * additional buffers. This way no buffers need to be garbage collected and
 * the contents don't have to be copied to the new buffer.
 */

Test build #111936 has started for PR 26085 at commit a601356.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/16951/
Test PASSed.

Test build #111936 has finished for PR 26085 at commit a601356.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/111936/
Test PASSed.

Test build #112193 has started for PR 26085 at commit 8dc8fad.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/17183/
Test PASSed.

Test build #112193 has finished for PR 26085 at commit 8dc8fad.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/112193/
Test FAILed.

Test build #112239 has started for PR 26085 at commit 0bf182a.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/17225/
Test PASSed.

Test build #112239 has finished for PR 26085 at commit 0bf182a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/112239/
Test PASSed.

Test build #112299 has started for PR 26085 at commit f184c4c.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/17283/
Test PASSed.

Test build #112300 has started for PR 26085 at commit bc6a14c.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/17284/
Test PASSed.

Test build #112301 has started for PR 26085 at commit 5aadd8f.

Hi, @dbtsai . I created a PR to your branch. Could you review and merge the updated benchmark result?

https://github.com/dbtsai/spark/pull/7

@dongjoon-hyun thanks. Merged into my branch.

Test build #112303 has started for PR 26085 at commit 08c8fb2.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/17286/
Test PASSed.

Test build #112299 has finished for PR 26085 at commit f184c4c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/112299/
Test FAILed.

Test build #112300 has finished for PR 26085 at commit bc6a14c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/112300/
Test PASSed.

Test build #112301 has finished for PR 26085 at commit 5aadd8f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/112301/
Test PASSed.

Test build #112303 has finished for PR 26085 at commit 08c8fb2.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/112303/
Test PASSed.

Thank you, @dbtsai , @tgravescs , @viirya , @MaxGekk , @advancedxy .
Merged to master.

sorry my late comment here, didn't have time last week to follow up, just curious if you tried like lz4 (or other) instead of zstd?  I would expect zstd to be a better choice but just wondering if you tried and what was performance diff or if it made sense to make this configurable in the future.

That sounds like a reasonable idea. Could you make a JIRA and a PR for the configuration, @tgravescs ? I can help you the benchmark. Thanks!

So does that mean you know lz4 (or others) weren't tried? I was curious if it was tried already before spending time on it.

No. I don't think it's tried. To make it sure, let's ping @dbtsai . :)

@tgravescs let me try lz4 quickly, and will post the result. Thanks.

@tgravescs The following the result ran on my desktop. LZ4 is 5x faster but creates 1.6x bigger data. Wondering should we trade the serialization time with larger data?

ZSTD

Java HotSpot(TM) 64-Bit Server VM 1.8.0_161-b12 on Mac OS X 10.14.2
Intel(R) Xeon(R) CPU E5-1650 v2 @ 3.50GHz
200000 MapOutputs, 1000 blocks w/o broadcast:  Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------------------------------
Serialization                                      3340           3355          21          0.1       16700.1       1.0X
Deserialization                                     650            660          14          0.3        3248.6       5.1X

Compressed Serialized MapStatus sizes: 123 MB
Compressed Serialized Broadcast MapStatus sizes: 0 bytes

LZ4

Running benchmark: 200000 MapOutputs, 1000 blocks w/o broadcast
  Running case: Serialization
  Stopped after 3 iterations, 2109 ms
  Running case: Deserialization
  Stopped after 5 iterations, 2424 ms

Java HotSpot(TM) 64-Bit Server VM 1.8.0_161-b12 on Mac OS X 10.14.2
Intel(R) Xeon(R) CPU E5-1650 v2 @ 3.50GHz
200000 MapOutputs, 1000 blocks w/o broadcast:  Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------------------------------
Serialization                                       677            703          32          0.3        3383.6       1.0X
Deserialization                                     466            485          27          0.4        2331.1       1.5X

Compressed Serialized MapStatus sizes: 194 MB
Compressed Serialized Broadcast MapStatus sizes: 0 bytes

LZF

Java HotSpot(TM) 64-Bit Server VM 1.8.0_161-b12 on Mac OS X 10.14.2
Intel(R) Xeon(R) CPU E5-1650 v2 @ 3.50GHz
200000 MapOutputs, 1000 blocks w/o broadcast:  Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------------------------------
Serialization                                      2199           2202           4          0.1       10994.6       1.0X
Deserialization                                     690            720          46          0.3        3450.6       3.2X

Compressed Serialized MapStatus sizes: 182 MB
Compressed Serialized Broadcast MapStatus sizes: 0 bytes

In this PR, ZSTD reduces the size like the following. If we choose LZ4, it seems to be a regression over the previous Apache Spark versions using Gzip. We need to distribute this file to all nodes, don't we? I guess that was the reason Apache Spark preferred GZIP before.
- Compressed Serialized MapStatus sizes: 131 MB
+ Compressed Serialized MapStatus sizes: 123 MB

thanks for running the tests @dbtsai. Its actually faster then I expected.
yes it needs to distribute it and this is obviously using memory on the driver side. Normally if the map status is of any size it will end up being broadcast to the hosts with the message going over the wire just indicating its a broadcast.  This to me isn't much different then any other broadcast thing which normally has the spark.io.compression.codec config applied for what compression to use.  You may actually want it faster if you have ample network.  I assume originally before we were broadcasting it, the size was definitely an issue because if it went over the max message size it would just fail. It also took a long time for large status' and slower networks.  That was also before we had highlycompressed status's and such as well.
This is definitely an improvement over what we had.  Perhaps we just wait and see if its an issue or if someone wants to use something other then zstd and at that point we an make it configurable if needed.  I hate to add more configs if not really needed.

Got it. Thanks, @tgravescs .

Agree, this PR already speeds up the serialization a bit, and unblocks our use-case. I was initially thinking to add another configuration, but also feel we have too many tuning parameters in Spark now. Let's see what other people think, and if they need to make it faster, we can consider the option to make it configurable.
Thanks.


Request: Microsoft Office Deployment Tool

Could also apply to anything else also on download.microsoft.com
I wish I knew RegEx, hopefully it's not too hard
Version on URL: https://www.microsoft.com/en-us/download/details.aspx?id=49117
i.e.
Version:

16.0.15629.20208

Download Link on URL: https://www.microsoft.com/en-us/download/confirmation.aspx?id=49117
i.e.
http-equiv="refresh" content="0;url=https://download.microsoft.com/download/2/7/A/27AF1BE6-DD20-4CB4-B154-EBAB8A7D4A7E/officedeploymenttool_15629-20208.exe"

Now merged into Dev and will be included in the next release.


sndio: initialize "par" to NULL

As discussed in #101, uninitialized use of par causes issues.

OK, I've got it all wrong this time...


Feature request: auto balancing

Similar to paredit.nvim and emacs paredit.el, having the option for auto balancing of Clojure data structures would be great.

Hi there.
Just so I can understand what you are asking for - are you referring to parenthesis balancing like what is depicted in https://shaunlebron.github.io/parinfer/, auto-pairing/bracket matching like from https://github.com/windwp/nvim-autopairs or something else entirely?

Thanks for the response. I had not seen autopairs before, and it appears it'll pair up nicely with your plugin. So far, enjoying it!

Hi @julienvincent, what are your thoughts on parinfer-like parenthesis balancing? Do you think there's a good reason for supporting it, or do I just need to get more used to paredit-style editing? (I mainly miss it when I want to comment out lines.) Thanks!


Package should be com.zhuinden.liveevent

Currently it is... com.zhuinden.liveeventsample.utils.
😠

Fixed in https://github.com/Zhuinden/live-event/commit/692c1382de5dc97b4ceb1a6b0f7657dbc3f99b49


adds help text to band input

Purpose
adds placeholder text to prompt the user to input band option, eg "Max (one or three bands, separated by commas)"
Related Issues
Closes COL-566
Submission Checklist

[x ] Included Jira issue in the PR title (e.g. COL-### Did something here)
[ x] Code passes linter rules for each file you updated. To lint all files at once, run npm run lint. To just lint one specific file run npx quick-lint-js src/js/<file-you-changed> --snarky.
[ x] No new reflection warnings (clojure -M:check-reflection)

Testing
Module Impacted
GeoDash
Role
Admin
Steps

navigate to a test project's "configure geodash" page. edit or create a new widget. select "Sentinel 2" as the Image Source. observe the helpful placeholder value of "min" and "max" fields

Desired Outcome
mild pleasant surprise

Screenshots

Hey Andy, can you modify the target branch for the PR from main to development?


Update proc-macro2 to fix compilation error with nightly

See https://github.com/rust-lang/rust/issues/113152 for more details.
This commit was generated by:
find . -name Cargo.toml -execdir cargo update -p proc-macro2 \;

@ia0 @panekj cargo install taplo-cli --locked still fails rn on nightly since no release has been made since this pr. could you do a release of taplo so that this fix is released?

Yes I'm also hitting this issue and having to work around it. And it's even more problematic since json_value_merge released a breaking change in 1.2.0 (https://github.com/jmfiaschi/json_value_merge/issues/18). I would suggest bumping that to 2.0.0 first then doing a release. I've create #473 to track that.


Show rule in math mode?

Is it possible? For example I want to show all "+" signs in red color or something similar.
Currently the following is possible -- define your own variable:
#let redplus= text(fill: red)[+]

$redplus^(redplus)$;

#[#set text(red); $+^+$]

Duplicate of #254


[Collections] Adjust cert verify date in tests

Motivation:
Tests in PackageCollectionsSigningTests that involve Test_ec.cer are failing. See https://github.com/apple/swift-package-manager/pull/3393.
I think this is a timezone-related issue. Test_ec.cer is valid from November 15, 2020 10:45:20am PST for a year. In the tests we manipulate the timestamp at which the cert validation is done so we don't have to regenerate new test certs and so we can test for different scenarios. That timestamp is set to November 16, 2020 midnight, and depending on where the tests are run, it might potentially mean a time before Test_ec.cer is valid and that's why we see "certificate is not yet valid" error. Note that this doesn't happen with tests that use Test_rsa.cer since the certificate is valid from November 8, 2020.
Modification:
Change verify date to November 18, 2020, which should give us more than enough buffer for various timezones.

@swift-ci please smoke test

@swift-ci please smoke test

I applied this patch and it fixed those three tests on Android, the different time zone was definitely the issue.


chore: init repo for essential Tanstack React Router packages

Hi! I'm the autofix.ci troubleshooter bot.
It looks like you correctly set up a CI job that uses the autofix.ci GitHub Action, but the autofix.ci GitHub App has not been installed for this repository. This means that autofix.ci unfortunately does not have the permissions to fix this pull request. If you are the repository owner, please install the app and then restart the CI workflow! 😃


skipWhen() doesn't work as expected if condition has been true once before

The skipWhen condition works as long as the first test hasErrors. After it has no errors anymore the second test runs. This is as expected so far.
But what I did not expect is that the second test continues to run with further inputs when the first test hasErrors again. I would expect that it gets skipped because the skipWhen condition is true again.
import { create, test, enforce, skipWhen } from 'vest';

export const signUpSuite = create((data = {}) => {
	test('someField', 'lt3', () => {
		enforce(data.someField).longerThan(3);
	});

	skipWhen(
		(res) => res.hasErrors('someField'),
		() => {
			test('someField', 'ends with x', () => {
				enforce(data.someField).endsWith('x');
			});
		}
	);
});

Environment:
Node: 16.11
Vest: 4.0.0-next-6abc96

Hey @bttger, thanks for reaching out.
I just tried to run a similar experiment and was unable to reproduce, but maybe I misunderstood the scenario.
Here's my sandbox: https://codesandbox.io/s/react-issues-737-xen0u?file=/src/suite.js
Not sure what framework you're using so I just used the react template. Can you fork the sandbox so it introduces the same result?
In the sandbox I added an alert any time the second test called. I didn't see the alerts firing the second time after the first test was invalid again.

The same thing happens in the sandbox that you provided. You can see it when joining the messages array in the components/Input/index.js file (e.g. {messages.join(" // ")} in line 19).

https://codesandbox.io/s/react-issues-737-forked-ocp8l?file=/src/components/Input/index.js
You can enter e.g. 'ddd' (error: too short), 'ddddd' (no error), 'dddddcat' (error: cat not allowed), 'dcat' (error: both errors, tho the cat error should be skipped)

Ah ok, then it makes sense why the alert is indeed being skipped but the errors still stay the same. And yes, I then agree that it would probably make more sense to handle it like an omit case rather than skipping evaluation and caching the errors. Yes, just let me know and I'll test it.

@bttger
I just released<EMAIL_ADDRESS>note that it is under the dev tag and not next. I also updated the sandbox to use this version: https://codesandbox.io/s/react-issues-737-xen0u?file=/src/suite.js
It seems like in our use-case, it works as expected, and matches your desired behavior. I still need to see how it behaves in other situations. This change broke a couple of Vest's unit tests, so I'm going to see if it is really misbehaving, or if it is just the state without the extra error that's breaking the test.
Either way, let me know if that works for you. If it does, it might end up in Vest soon (or we'll find another way to introduce test omission that will satisfy this use case).
Thank you for your feedback.

Thank you for implementing it so fast. I have installed the new version and my forms are working as expected now. In my opinion, this is now the behavior I'd expect from skipWhen and I'd love to see it in Vest 4. Thanks for your work!
I've run the tests locally to see which ones are affected but I can't really see how they are correlated :sweat_smile:

@bttger thanks for trying this and reporting back. Your feedback is very helpful in determining the future of this API.
I did find the odd case in which this change doesn't play nicely. Here's a sandbox: https://codesandbox.io/s/react-skipwhen-omits-knm31?file=/src/suite.js
Imagine you have a legitimate case for skipping a field during runtime, yet you don't want to completely omit the test, as it would mark it as optional. For example in the sandbox, I wrapped the confirmation test with a skipWhen so it doesn't get validated when the password is invalid to begin with. For as long as the password is invalid, the confirmation is optional, and the form can be submitted.
This can lead to the opposite side of the confusion that you described. When people wish skipWhen to be a focused skip.
I am starting to feel that in terms of design, an applicable solution would be to keep skipWhen as it is (focused skip), but add one of the following:
omitWhen() wrapper
A comparable API to skipWhen. It does exactly what you need, but will retain the ability to satisfy the need for focused-skip.
omitWhen(res => res.hasErrors('username'), () => {
  test(...)
});

omit() modifier
A complementary  API to skipWhen and group. Calling it within any block in Vest, will omit all the tests within that block. It adheres to the conditional specified in skipWhen, so if it is falsy, it won't have any effect.
skipWhen(res => res.hasErrors('username'), () => {
  omit(); // this will mark all the tests within this block as omitted and optional.

  test(...)
});

Both options are equally powerful, and each has its pros and cons. Either way they will reduce most of the ambiguity, because it makes sense that skipWhen is a "sibling" of skip, and that omit (or omitWhen) completely omits the test from the suite.
If you had to use one, which feels more ergonomic to you?
As a side note:
The omit() modifier option makes me think of this issue: https://github.com/ealush/vest/issues/733 where the request was for a similar execution modifier, for eager returns from blocks. I am wondering if there can be other requested modifiers, or these provide the basic building blocks for a suite.

Imagine you have a legitimate case for skipping a field during runtime, yet you don't want to completely omit the test, as it would mark it as optional. For example in the sandbox, I wrapped the confirmation test with a skipWhen so it doesn't get validated when the password is invalid to begin with. For as long as the password is invalid, the confirmation is optional, and the form can be submitted.
This can lead to the opposite side of the confusion that you described. When people wish skipWhen to be a focused skip.

Yes that's right. The odd case is now that the skipWhen condition is put on another field which doesn't get tested because of the only() call. (Though this suite wouldn't work in real life anyways because I can modify the password again after I have inserted the confirmation. They don't equal anymore but the suite is still valid if the confirmation input was correct once.)

Both options are equally powerful, and each has its pros and cons. Either way they will reduce most of the ambiguity, because it makes sense that skipWhen is a "sibling" of skip, and that omit (or omitWhen) completely omits the test from the suite.

I support this. omitWhen feels semantically correct when you have the intention to omit the test case (and the corresponding message) from the suite result if a condition is met during a test run.
Personally I would prefer omitWhen over the omit call inside of another block. I just think the readability is better and since it would have the same signature as the skipWhen function.
Now that I think of it, I guess it could always create some odd cases if you apply a skipWhen or omitWhen condition on a field other than the one that is currently running (due to the only() call). But maybe I miss something.

@bttger
Thanks for the quick feedback. This really helps me iterate quickly on these ideas.
I released 4.0.0-dev-cc2a13 which introduces omitWhen, and retains skipWhen as it is. I'll add it to vest@next release once I write the tests and update the documentation for it. In the meantime, can you please let me know if it works the way you intend it to?
Here you can see it running on the sandbox: https://codesandbox.io/s/react-issues-737-xen0u?file=/src/suite.js

Now that I think of it, I guess it could always create some odd cases if you apply a skipWhen or omitWhen condition on a field other than the one that is currently running and if they are dependent (due to the only() call). But maybe I miss something.

Yes, I agree, there are always some odd cases on non-focused fields/tests, but most of these cases self correct very easily once the user starts goes back to the field, tries to submit (to validate all).
In case this is mission-critical that these edge cases never happen, a consumer of Vest can take an extra step and make sure these don't happen on their own end by creating more specific skip/omission rules, or even going further and validating multiple fields together, for example:
only(currentField);
if (currentField === "password") only("confirm");

and then
skipWhen(res=>res.hasErrors('password'), () => {
  test('confirm', ...)
});

This will make both fields validate together, unless the password is invalid. Not perfect, but it demonstrates that things can be done to avoid common issues.

Hm. That idea lead me to think. It might be useful to add a linked fields utility to Vest, that could tell it to validate multiple fields together on some conditions.
import link from 'vest/link';

link.when('password').
	add('confirm');

But this one can wait.

BTW, here's the change that supports omitWhen if you're interested :
https://github.com/ealush/vest/pull/738

If you now install vest@next you'll have access to omitWhen, I also updated the docs

Hm. That idea lead me to think. It might be useful to add a linked fields utility to Vest, that could tell it to validate multiple fields together on some conditions.

Good idea, though I wouldn't need it. As you said, I am validating all fields together anyways before submitting. But maybe someone else has the need for it.
Thank you for the quick omitWhen extension. I've updated to the next branch and only had to rename the skipWhen occurrences. It works nice now :)
I just created a PR (https://github.com/ealush/vest-website/pull/1) for explicit mention about the effects on the validation message in the docs.

Thanks for that. I merged your PR over there 🙏🏻
Also, thank you for all the help and helping me quickly iterate over solutions.
Closing this issue for now :)


RefreshIndicator needs an audit and an overhaul to make it behave like the native indicator

Steps to reproduce
The Flutter RefreshIndicator does not behave like the native refresh indicator on Android (e.g. as seen in the Gmail app).
I am creating this issue as an umbrella issue to cover the different aspects of this. Really RefreshIndicator needs a careful audit and overhaul to bring it in line with the behavior of the native refresh indicator on Android (and also iOS, but I know nothing about that).
I gave detail on the context in this comment: https://github.com/flutter/flutter/pull/139535#issuecomment-1874650957
In summary:
The Android native refresh indicator changes color when it is armed, and can be dragged a further distance in armed state, which gives a clear indication that the indicator is armed. It also allows a user to drag back to above the arming point, to disarm the indicator again before releasing.
Flutter doesn't allow that -- there is no indication of armed state (#139018), and there is no ability to drag beyond the arming point. Also, in Flutter, after arming, the indicator can end up disarmed if the user moves up by even one pixel (https://github.com/flutter/flutter/issues/138848). It is very easy to trigger this, because there often is a "bounceback" effect on touchpoint-up, whereby a small upwards scroll is initiated by Flutter, even if the user didn't drag upwards (also described in #138848).
Even the visual appearance of the Flutter indicator doesn't properly match the native indicator.
I fixed the lack of overdrag in #139535. However, this is only a partial fix. More work needs to be done to make the Flutter indicator look and work like the native indicator. Hence this issue.
Expected results
See above
Actual results
See above
Code sample
N/A
Screenshots or Video
No response
Logs
No response
Flutter Doctor output
Doctor output
$ flutter doctor -v
[✓] Flutter (Channel beta, 3.18.0-0.2.pre, on Fedora Linux 39 (Workstation Edition) 6.5.12-300.fc39.x86_64, locale en_US.utf8)
    • Flutter version 3.18.0-0.2.pre on channel beta at /opt/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision fed06b31d9 (3 weeks ago), 2023-12-13 14:53:33 -0800
    • Engine revision 39c6dc9bdd
    • Dart version 3.3.0 (build 3.3.0-174.3.beta)
    • DevTools version 2.30.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.1)
    • Android SDK at /home/luke/Android/Sdk
    • Platform android-34, build-tools 33.0.1
    • Java binary at: /opt/android-studio/jbr/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at google-chrome

[✓] Linux toolchain - develop for Linux desktop
    • clang version 17.0.4 (Fedora 17.0.4-1.fc39)
    • cmake version 3.27.7
    • ninja version 1.11.1
    • pkg-config version 1.9.5

[✓] Android Studio (version 2022.2)
    • Android Studio at /opt/android-studio
    • Flutter plugin version 73.0.2
    • Dart plugin version 222.4582
    • Java version OpenJDK Runtime Environment (build 17.0.6+0-17.0.6b802.4-9586694)

[✓] VS Code (version 1.85.1)
    • VS Code at /usr/share/code
    • Flutter extension version 3.81.20231227

[✓] VS Code (version 1.86.0-insider)
    • VS Code at /usr/share/code-insiders
    • Flutter extension version 3.79.20231130

[✓] Connected device (3 available)
    • sdk gphone x86 64 (mobile) • emulator-5554 • android-x64    • Android 13 (API 33) (emulator)
    • Linux (desktop)            • linux         • linux-x64      • Fedora Linux 39 (Workstation Edition) 6.5.12-300.fc39.x86_64
    • Chrome (web)               • chrome        • web-javascript • Google Chrome 119.0.6045.159

[✓] Network resources
    • All expected network resources are available.

• No issues found!

Thanks for the detailed report @lukehutch, but I think your issue is very similar to an already open umbrella issue : https://github.com/flutter/flutter/issues/96528, so I'll close this in favor of it. You may share all your findings in it for better tracking.
If you disagree, write in comments and I'll reopen it.


✨ Commit depth feature

What kind of change does this PR introduce?
Feature to implement issue to add a flag for --commit-depth : https://github.com/ossf/scorecard/issues/2224

[x] PR title follows the guidelines defined in our pull request documentation

What is the current behavior?
No --commit-depth flag exists to modify number of commits to use in checks.
What is the new behavior (if this is a feature change)?**
--commit-depth flag is added, which allows the number of commits to be used in checks to be increased/decreased.

[ ] Tests for the changes have been added (for bug fixes/features)

Which issue(s) this PR fixes
https://github.com/ossf/scorecard/issues/2224

Special notes for your reviewer
Does this PR introduce a user-facing change?
For user-facing changes, please add a concise, human-readable release note to
the release-note
(In particular, describe what changes users might need to make in their
application as a result of this pull request.)

Added --commit-depth optional flag to allow users to configure the amount of commits scorecard analyzes.

Maybe add some unit tests. Also think of adding the results in the final results (pkg/json_results.go) and pkg/raw_results...
@laurentsimon
for unit tests, where should they be placed? in graphql_e2e_test.go?
or a new file like graphql_test.go i dont see a file that contains tests for other functions like commitsFrom for example.

Maybe add some unit tests. Also think of adding the results in the final results (pkg/json_results.go) and pkg/raw_results...

@laurentsimon for unit tests, where should they be placed? in graphql_e2e_test.go?

I think this file should be OK. You may need a demo repo that never changes so we can keep testing the same thing over time. Shall I create a repo under https://github.com/ossf-tests/? Maybe scorecard-check-graphql-e2e?

Maybe add some unit tests. Also think of adding the results in the final results (pkg/json_results.go) and pkg/raw_results...

@laurentsimon for unit tests, where should they be placed? in graphql_e2e_test.go?

I think this file should be OK. You may need a demo repo that never changes so we can keep testing the same thing over time. Shall I create a repo under https://github.com/ossf-tests/? Maybe scorecard-check-ghclient-graphql-e2e?

Sure but before you do that, can you check the following changes i added to the e2e_test.go file. I added some tests for the populateCommits function but ive never used this testing framework so i could have made mistakes. I just added a new context and have my tests in there. Does that work?
var _ = Describe("E2E TEST: githubrepo.graphqlHandler", func() {
	var graphqlhandler *graphqlHandler

	BeforeEach(func() {
		graphqlhandler = &graphqlHandler{
			client: graphClient,
		}
	})
        // -> my test below
	Context("E2E TEST: Confirm Paging Commits Works", func() {
		It("Should only have 1 commit", func() {
			_repourl := &repoURL{
				owner:     "ossf",
				repo:      "scorecard",
				commitSHA: clients.HeadSHA,
			}
			_vars := map[string]interface{}{
				"owner":                  githubv4.String("ossf"),
				"name":                   githubv4.String("scorecard"),
				"pullRequestsToAnalyze":  githubv4.Int(1),
				"issuesToAnalyze":        githubv4.Int(30),
				"issueCommentsToAnalyze": githubv4.Int(30),
				"reviewsToAnalyze":       githubv4.Int(30),
				"labelsToAnalyze":        githubv4.Int(30),
				"commitsToAnalyze":       githubv4.Int(1),
				"commitExpression":       githubv4.String("heads/main"),
				"historyCursor":          (*githubv4.String)(nil),
			}
			_ctx := context.Background()
			_logger := log.NewLogger(log.DebugLevel)
			_rt := roundtripper.NewTransport(_ctx, _logger)
			_httpClient := &http.Client{
				Transport: _rt,
			}
			_graphClient := githubv4.NewClient(_httpClient)
			_handler := &graphqlHandler{
				client: _graphClient,
			}
			_handler.init(context.Background(), _repourl)
			commits, err := populateCommits(_handler, _vars)
			Expect(err).To(BeNil())
			Expect(len(commits)).Should(BeEquivalentTo(1))
		})
	})

	Context("E2E TEST: Validate query cost", func() {
		It("Should not have increased for HEAD query", func() {
			repourl := &repoURL{
				owner:     "ossf",
				repo:      "scorecard",
				commitSHA: clients.HeadSHA,
			}
			graphqlhandler.init(context.Background(), repourl)
			Expect(graphqlhandler.setup()).Should(BeNil())
			Expect(graphqlhandler.data).ShouldNot(BeNil())
			Expect(graphqlhandler.data.RateLimit.Cost).ShouldNot(BeNil())
			Expect(*graphqlhandler.data.RateLimit.Cost).Should(BeNumerically("<=", 1))
		})
		It("Should not have increased for commit query", func() {
			repourl := &repoURL{
				owner:     "ossf",
				repo:      "scorecard",
				commitSHA: "de5224bbc56eceb7a25aece55d2d53bbc561ed2d",
			}
			graphqlhandler.init(context.Background(), repourl)
			Expect(graphqlhandler.setup()).Should(BeNil())
			Expect(graphqlhandler.data).ShouldNot(BeNil())
			Expect(graphqlhandler.data.RateLimit.Cost).ShouldNot(BeNil())
			Expect(*graphqlhandler.data.RateLimit.Cost).Should(BeNumerically("<=", 1))
		})
		It("Should not have increased for check run query", func() {
			repourl := &repoURL{
				owner:     "ossf",
				repo:      "scorecard",
				commitSHA: clients.HeadSHA,
			}
			graphqlhandler.init(context.Background(), repourl)
			Expect(graphqlhandler.setupCheckRuns()).Should(BeNil())
			Expect(graphqlhandler.checkData).ShouldNot(BeNil())
			Expect(graphqlhandler.checkData.RateLimit.Cost).ShouldNot(BeNil())
			Expect(*graphqlhandler.checkData.RateLimit.Cost).Should(BeNumerically("<=", 1))
		})
	})
})

LGTM at a high-level. Let's see if the tests pass :)
Fyi, not an expert either on the testing framework :)

LGTM at a high-level. Let's see if the tests pass :) Fyi, not an expert either on the testing framework :)

Are there next steps ?
Also there is an issue with 3 checks specifically the SAST, CI-BestPracticesCheck and CI-Tests Checks. When the number of commits is set to greater than 100, those checks will print a cache miss informational error. due to the setupCheckRuns function being called. And that number not matching the number of commits saved by the client in the setup command.
func (handler *graphqlHandler) setupCheckRuns() error {
	handler.setupCheckRunsOnce.Do(func() {
		commitExpression := handler.commitExpression()
		vars := map[string]interface{}{
			"owner":                 githubv4.String(handler.repourl.owner),
			"name":                  githubv4.String(handler.repourl.repo),
			"pullRequestsToAnalyze": githubv4.Int(pullRequestsToAnalyze),
			"commitsToAnalyze":      githubv4.Int(CommitsToAnalyze),
			"commitExpression":      githubv4.String(commitExpression),
			"checksToAnalyze":       githubv4.Int(checksToAnalyze),
		}
		// TODO:
		// sast and ci checks causes cache miss if commits dont match number of check runs.
		// paging for this needs to be implemented if using higher than 100 --number-of-commits
		if CommitsToAnalyze > 99 { // <- paging not supported to maxed out to 99
			vars["commitsToAnalyze"] = githubv4.Int(99)
		}
		if err := handler.client.Query(handler.ctx, handler.checkData, vars); err != nil {
			// quit early without setting crsErrSetup for "Resource not accessible by integration" error
			// for whatever reason, this check doesn't work with a GITHUB_TOKEN, only a PAT
			if strings.Contains(err.Error(), "Resource not accessible by integration") {
				return
			}
			handler.errSetupCheckRuns = err
			return
		}
		handler.checkRuns = parseCheckRuns(handler.checkData)
	})
	return handler.errSetupCheckRuns
}

to fix this i think the graphql query for setupcheckruns would need to be rewritten to have paging. But currently it doesnt seem to be a big issue.

Are there next steps ?
Also there is an issue with 3 checks specifically the SAST, CI-BestPracticesCheck and CI-Tests Checks. When the number of commits is set to greater than 100, those checks will print a cache miss informational error. due to the setupCheckRuns function being called. And that number not matching the number of commits saved by the client in the setup command.

Thanks for pointing this out. Ok to fix this cache miss in a follow-up PR and submit this PR with its current functionality. My only ask would be to leave #2224 open so that we can track this work. Also, if you would like to work on this follow-up too let us know by commenting on #2224, else one of the maintainers or someone else can take this up.

Looks good to merge.

@azeemshaikh38 let me know if that is ok, all the tests are passing and seems to work fine, i added 0 in all cases where 30 was and used <= instead of ==
i thought i signed the commit, but i guess it didnt so it told me to rebase. that causes conflicts im guessing lmk what i need to do to fix any problems Thanks!

Are there next steps ?
Also there is an issue with 3 checks specifically the SAST, CI-BestPracticesCheck and CI-Tests Checks. When the number of commits is set to greater than 100, those checks will print a cache miss informational error. due to the setupCheckRuns function being called. And that number not matching the number of commits saved by the client in the setup command.

Thanks for pointing this out. Ok to fix this cache miss in a follow-up PR and submit this PR with its current functionality. My only ask would be to leave #2224 open so that we can track this work. Also, if you would like to work on this follow-up too let us know by commenting on #2224, else one of the maintainers or someone else can take this up.

ill probably take a shot a it. may need help will ask in the issue

i thought i signed the commit, but i guess it didnt so it told me to rebase. that causes conflicts im guessing lmk what i need to do to fix any problems Thanks!

Thanks. We need to resolve the conflicts before merging. Could you please do that? I'm also looking into it on my side.

i thought i signed the commit, but i guess it didnt so it told me to rebase. that causes conflicts im guessing lmk what i need to do to fix any problems Thanks!

Thanks. We need to resolve the conflicts before merging. Could you please do that? I'm also looking into it on my side.

Just completed. lmk if there are any issues!

i thought i signed the commit, but i guess it didnt so it told me to rebase. that causes conflicts im guessing lmk what i need to do to fix any problems Thanks!

Thanks. We need to resolve the conflicts before merging. Could you please do that? I'm also looking into it on my side.

Just completed. lmk if there are any issues!

Thanks a lot! Very much appreciate your effort on this PR :) Will work on getting this merged.

FYI the change in pkg/scorecard.go was a breaking change, should've gone into a major semver


KeyError: 'num' occurs while tokenization.

import spacy
nlp = spacy.load('ja_ginza_nopn')

doc = nlp('2016.9 - 2017.2')

for token in doc:
    print(token.text)

I'm using Ginza to split Japanse sentences into tokens.
And, the error below occured.
The snippet above is the minimal code which reproduces the error I bumped into.
Does anyone know the way to solve this problem?
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-23-9a0d074dcdf0> in <module>
      2 nlp = spacy.load('ja_ginza_nopn')
      3
----> 4 doc = nlp('2016.9 - 2017.2')
      5
      6 for token in doc:

~/miniconda3/envs/shinra/lib/python3.6/site-packages/spacy/language.py in __call__(self, text, disable, component_cfg)
    383             if not hasattr(proc, "__call__"):
    384                 raise ValueError(Errors.E003.format(component=type(proc), name=name))
--> 385             doc = proc(doc, **component_cfg.get(name, {}))
    386             if doc is None:
    387                 raise ValueError(Errors.E005.format(name=name))

~/miniconda3/envs/shinra/lib/python3.6/site-packages/ginza/japanese_corrector.py in __call__(self, doc)
     18
     19     def __call__(self, doc):
---> 20         correct_dep(doc)
     21         set_bunsetu_bi_type(doc)
     22         return doc

~/miniconda3/envs/shinra/lib/python3.6/site-packages/ginza/japanese_corrector.py in correct_dep(doc)
     61                     except ValueError:
     62                         pass
---> 63                 token.head.pos_ = corrected_pos
     64                 token.pos_ = corrected_pos
     65                 token.dep_ = 'dep'

token.pyx in spacy.tokens.token.Token.pos_.__set__()

KeyError: 'num'

When I feed  '2016.9 - 2017.' instead of '2016.9 - 2017.2', the error won't occur.
Also, there is no problem with '2017.2' .
I tried 'en' package and it worked with the sentence '2016.9 - 2017.2'.

@dwarfer7634 Thanks for reporting the error. I'm going to fix it.

@dwarfer7634 Thanks for reporting the problem. This had been fixed by v2.0.0. Please install the latest module and model, then load the model by executing nlp = spacy.load('ja_ginza').

Thank you very much!


Move post dates to left in index

Hey @farhadmpr,
Thanks for your PR, But unfortunately I can't merge it. I get the idea of posts list from zenhabits.net and I prefer to stick to it.


No output for some packages

Some packages result in silent failures. One example:
$ pipdeptree -p qgrid

$

This package actually has a quite a large dependency tree.

Is the issue that qgrid indirectly reads its requirements from a file? See here:
https://github.com/quantopian/qgrid/blob/master/setup.py#L146
FWIW, I can get the (huge) dep tree with johnnydep, but it takes forever, so I expect it is doing something like recursively executing the various {{setup.py}} files to do it.

Seems there hasn't been any push for this for 3 years, so closing for now.


Include invalid property name in the error message for 'minimum' and 'maximum' validators

Hi @Julian, thanks for this library and all the continuous work you put into it! 🥇
We encountered a small inconvenience when checking the error message for properties that are not within a min/max range. It does not contain the name of the property, but only its value, making it difficult to pinpoint which property is invalid.
def minimum(validator, minimum, instance, schema):
    if not validator.is_type(instance, "number"):
        return

    if instance < minimum:
        message = f"{instance!r} is less than the minimum of {minimum!r}"
        yield ValidationError(message)

def maximum(validator, maximum, instance, schema):
    if not validator.is_type(instance, "number"):
        return

    if instance > maximum:
        message = f"{instance!r} is greater than the maximum of {maximum!r}"
        yield ValidationError(message)

Current message: 12 is less than the minimum of 20
Suggested message: 12 is less than the minimun of 20 for 'my_random_property_name'
Is this something that you consider improving?

Hello, thanks for the quick reply!
I actually  used absolute_path to extract the name (not sure if it's the right property to use). I was just wondering if this can be avoided and if the property name being included in the error message would be useful for others as well.