[google_maps] fix markersToChange to actually change modified markers

Description
Fixes Marker updating method.
now markers needed to change are actually those which need to be changed
Related Issues
flutter/flutter#27823
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

[X ] I read the Contributor Guide and followed the process outlined there for submitting PRs.
[ ] My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
[ ] All existing and new tests are passing.
[ ] I updated/added relevant documentation (doc comments with ///).
[x ] The analyzer (flutter analyze) does not report any problems on my PR.
[x] I read and followed the Flutter Style Guide.
[x] The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
[ ] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
[ ] I updated CHANGELOG.md to add a description of the change.
[ ] I signed the CLA.
[x] I am willing to follow-up on review comments in a timely manner.

Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?

[ ] Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
[x] No, this is not a breaking change.

I signed it!

Hello @amirh, can you update us on the status on this one please? Thank you.

@amirh What is the status of this PR?

Why is this taking so long? This is a pretty important performance fix.

Is there something I can do to speed up processing of the PR?

@iskakaushik

Left a few comments, Note that multiple tests and presubmits are failing, they will need to be fixed before a PR can be merged.
Generally speaking we are prioritizing issues by the number of thumbs up, so the best way to get it prioritize is to thumb up flutter/flutter#27823. See the review process section.

@amirh I've now changed code accordingly to above comments. Thanks for the tip regarding _markersToChange. There is a noticeable performance improvement when changing lot of markers at once.

@amirh @iskakaushik  can you check if this is ok? We need google map to be as fast as possible. Thanks

Fixed in https://github.com/flutter/plugins/pull/1933


chore: set in app include so sentry detects our application code

Sentry currently detects stack traces for renku code as "system" instead of in-app. This also means it doesn't link to github source code and treats errors as 3rd party library errors.
/deploy #notest extra-values=dataService.sentry.enabled=true,dataService.sentry.dsn=https://e47c95d665684bce89984bd4595268df@sentry.dev.renku.ch/13,dataService.sentry.environment=ci-renku-3548,dataService.sentry.sampleRate=0.5

You can access the deployment of this PR at https://renku-ci-ds-498.dev.renku.ch

Pull Request Test Coverage Report for Build<PHONE_NUMBER>1
Details

0 of 0   changed or added relevant lines in 0 files are covered.
2 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.02%) to 90.836%

Files with Coverage Reduction
New Missed Lines
%

components/renku_data_services/crc/blueprints.py
1
93.19%

components/renku_data_services/storage/blueprints.py
1
93.94%

Totals

Change from base Build<PHONE_NUMBER>3:
0.02%

Covered Lines:
11111

Relevant Lines:
12232

💛  - Coveralls

yes. e.g. https://sentry.dev.renku.ch/organizations/sentry/issues/8548/?project=13&query=is%3Aunresolved&referrer=issue-stream is how things are currently and you can see that our code has the "system" tag (top right):

and with this PR https://sentry.dev.renku.ch/organizations/sentry/issues/8611/?project=13&query=is%3Aunresolved&referrer=issue-stream

now says "in app" in the top right and has an "open line in github" link.
Note that the latter only works for code in components/ as sentry want's to map errors to code 1:1 and I've mapped import renku_data_services.* to components/renku_data_services/*, which makes the most sense to me as that's the likeliest place for errors to occur and we can't have more finegrained mappings (or well, we'd have to manually map each module which doesn't seem worth the effort)

Tearing down the temporary RenkuLab deplyoment for this PR.


test: Only wait for one operator instance to be ready

Waiting for only one Cilium operator instance to be ready allows
testing in single node environments, such as Minikube.
Signed-off-by: Jarno Rajahalme<EMAIL_ADDRESS>

test-me-please

test-me-please

test-me-please

known flakes (#14463 and #14125) on runtime tests, no other failures.

retest-runtime


Cache encryption

Is there a way to apply encryption to the file saved on the disk?
I know about the data that are stored inside but is there a way to actually encrypt the file?

Please take a look at the subclassing notes in Cache.swift. I think you could simply subclass the Cache and provide your own means of encryption/decryption before calling super.

I see thanks.


gh label list Fails Within Repository

Hi @heaths,
I've recently downloaded your extension and when I tried to list the labels I received the following error,
error: cannot find gh; is it installed? error: exec: "gh": executable file not found in $PATH

However, I am able to use gh normally and gh label --help works. Here is my path which contains gh twice (exported again to try and pix this issue)
/usr/bin/latexmk:/snap/bin/gh:/usr/bin/latexmk:/home/jordan/anaconda3/bin:/home/jordan/anaconda3/condabin:/home/jordan/.opam/default/bin:/home/jordan/.cargo/bin:/home/jordan/.local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:~/Documents/flutter/bin:~/Documents/flutter/bin:/snap/bin/gh

Regards
Jordam

How did you download it? Did you run gh extension install heaths/gh-label? That's the only method supported for downloading gh extensions.
Note, however, this really isn't needed anymore. There's a built-in gh label and I added what functionality this did have it didn't to the built-in gh label several versions ago.

Yes I did use gh extension install heaths/gh-label. I expect my gh is out of date is all. I'll try updating it. :+1:


Creates a MongoDBSource

By utilizing the MongoDB ChangeStreams (https://www.mongodb.com/docs/manual/changeStreams/) we are able to capture change events and emit them as Cloudevents.

@JeffNeff could you please take a look at the CI failures on this PR

Please squash PRs before merge.


Team Member Code Review

Code Review
Results summary
Great job getting the functionality working! I can add, delete, and mark todos as complete. Well done.
Adding new todos also keeps the existing todos checked!
In smaller browsers / mobile phones it's very difficult to use the website because the app shrinks very small (screenshot below).
There are also a few HTML errors that are being flagged by the validator.
Checklist

[x] The web app fulfills the minimum functional requirements in the app instructions section
[x] GitHub Pages is activated in the repo and the web app is live
[ ] The HTML index.html page passes W3C HTML validation
[ ] The web app passes W3C CSS validation

Responsive issue

Validation errors
HTML errors:
https://validator.w3.org/nu/?doc=https%3A%2F%2Fimd1005-web-development-winter-2023.github.io%2Fassignment-03-Fuzzyspuffyy%2F
One small CSS error:
https://jigsaw.w3.org/css-validator/validator?uri=https%3A%2F%2Fimd1005-web-development-winter-2023.github.io%2Fassignment-03-Fuzzyspuffyy%2F&profile=css3svg&usermedium=all&warning=1&vextwarning=&lang=en
Screenshot

https://www.youtube.com/watch?v=HHtMMS42cWs&t=15s
I am not sure if using this video to make the pg responsive it correct. @neilmispelaar


EscolaLmsAuthServiceProvider included in TestCase

It should fix https://github.com/EscolaLMS/API/pull/310


Notification widget ui is slow

from google play user:
Reaction on touching a volume bar is not instant

should be improved in 2.3.4


TLS handshake continue

Is there a way to make cowboy fetch a certificate by calling some custom code at the time of each TLS handshake?
There is the ssl option {sni_fun, fun()} but it is not called if the client doesn't send SNI.
Some interface using handshake hello/continue would probably work, I guess.

You need to write a custom Ranch transport and use it instead of ranch_ssl.

It's been added to ranch_ssl already in https://github.com/ninenines/ranch/pull/271.
I think it's rather cowboy_tls which needs to be adapted slightly, specifically the line {ok, Socket} = ranch:handshake(Ref), because ranch:handshake/1 may actually return {continue, _} here if {hanshake, hello} is specified in the TLS options.
I was thinking about passing an option {handshake_continue_fun, fun(...) -> {continue, TlsOptions} | cancel} through to cowboy_tls, which would be called if ranch:handshake/1 returns a continue tuple, but since you closed the issue, I assume an official addition for this use case isn't wanted in cowboy. Is that right?

Yes in Ranch 2.0 it'll allow using the multi step handshake, but I'm not sure Cowboy should do anything about it. You're the first to ask. I'm also not sure what Cowboy should do about options that require Ranch 2.0.
Simpler solution for your case would probably to have a custom transport on top of ranch_ssl, forwarding all calls except for the handshake, but it's also possible to use a custom module instead of cowboy_tls. Or patching Cowboy of course.
If the feature is more popular or there's commercial interest I can consider adding it, and of course PRs are always welcome, but my schedule is fairly limited due to current events so it'll take a while before it may be merged.


Pentadactyl + JavaScript: getting value of an object?

Version:
Pentadactyl hg7290 (created 2016/07/19 07:00:08) running on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:45.0) Gecko/20100101 Firefox/45.0

Firefox.app in question is actually Firefox Extended Support Release, which still seems to be the latest Firefox available.
Issue:
Let's say I open a random issue page on a GitHub tracker (like this one), then I open Console in Firefox and run something like this:
alert(document.getElementsByClassName("js-issue-title")["0"]["firstChild"].textContent)

As a result, it pops me up with the title of an issue (speaking of this particular issue, it would be How to achieve Ctrl-t functionality).
Now, I want to use that value in a script with Pentadactyl. Thus, I add something like this to my .pentadactylrc
:javascript <<EOF
function dummyFu() {
    var myElement = document.getElementsByClassName("question-hyperlink");
    alert(myElement["0"].firstChild.textContent);
}
EOF
map -silent <F5> -js dummyFu()

However, when run, it just says TypeError: myElement[0] is undefined.
I'm do not know much of JavaScript, unfortunately, so there's probably some mistake in my thinking?  I just want to be able to get an element from a page by a class and then use it my script... Would be grateful for any suggestion, thanks.

See https://developer.mozilla.org/en-US/docs/Web/API/Window/content


Head block overlapping: a bug that affects history but not current version of prometheus

What did you do?
Prometheus Version: 2.25 (Apologies for using this old version and bringing up an issue about it)
I took a snapshot of Prometheus and used it to migrate data from one machine to another, and got these kinds of errors:
level=error ts=2022-12-10T13:05:25.321Z caller=db.go:766 component=tsdb msg="compaction failed" err="compact head: reloadBlocks blocks: invalid block sequence: block time ranges overlap: [mint:<PHONE_NUMBER>883, maxt:<PHONE_NUMBER>106, range: 30m41s, blocks: 2]: <ulid: 01GK166NFQHD6NTVNASWCQ5HY5, mint:<PHONE_NUMBER>235, maxt:<PHONE_NUMBER>106, range: 1h56m25s>, <ulid: 01GKY27NJPEP56Q4B40SW0EZKJ, mint:<PHONE_NUMBER>883, maxt:<PHONE_NUMBER>000, range: 34m15s>

After a few days, Prometheus consumed hundreds of Gigabytes of disk space.
umon      23730 23883 actiontech-universe   41r      REG               0,56   22628589  104990728 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/chunks/000001 (deleted)
umon      23730 23883 actiontech-universe   92r      REG               0,56    2489901  100684698 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/index (deleted)
umon      23730 23884 actiontech-universe   41r      REG               0,56   22628589  104990728 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/chunks/000001 (deleted)
umon      23730 23884 actiontech-universe   92r      REG               0,56    2489901  100684698 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/index (deleted)
umon      23730 23885 actiontech-universe   41r      REG               0,56   22628589  104990728 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/chunks/000001 (deleted)
umon      23730 23885 actiontech-universe   92r      REG               0,56    2489901  100684698 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/index (deleted)
umon      23730 23886 actiontech-universe   41r      REG               0,56   22628589  104990728 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/chunks/000001 (deleted)
umon      23730 23886 actiontech-universe   92r      REG               0,56    2489901  100684698 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/index (deleted)
umon      23730 23887 actiontech-universe   41r      REG               0,56   22628589  104990728 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/chunks/000001 (deleted)
umon      23730 23887 actiontech-universe   92r      REG               0,56    2489901  100684698 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/index (deleted)
umon      23730 23900 actiontech-universe   41r      REG               0,56   22628589  104990728 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/chunks/000001 (deleted)
umon      23730 23900 actiontech-universe   92r      REG               0,56    2489901  100684698 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/index (deleted)
umon      23730 27339 actiontech-universe   41r      REG               0,56   22628589  104990728 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/chunks/000001 (deleted)
umon      23730 27339 actiontech-universe   92r      REG               0,56    2489901  100684698 /opt/umon/prometheus-data/01GSE6Y24AE3FZAJK7G1779GST/index (deleted)

It seems that Prometheus opened a head block but didn't close it. Deletion in Linux simply unlinks a file, the linux kernel maintains link count for each i-node and also counts open file descriptors for that file, if the file is still open, the file wouldn't get deleted.
Maybe a fix should be like adding db.Close():
if err := db.reloadBlocks(); err != nil {
		if errRemoveAll := os.RemoveAll(filepath.Join(db.dir, uid.String())); errRemoveAll != nil {
			return tsdb_errors.NewMulti(
				db.Close(),  <= THIS LINE IS ADDED
				errors.Wrap(err, "reloadBlocks blocks"),
				errors.Wrapf(errRemoveAll, "delete persisted head block after failed db reloadBlocks:%s", uid),
			).Err()
		}
		return errors.Wrap(err, "reloadBlocks blocks")
	}

Since the newest version has already removed the  storage.tsdb.allow-overlapping-blocks option and it's enabled now, this issue seems to be trivial.
Should there be any fix against it?
What did you expect to see?
Disk usage shouldn't be keep going up, as a result, the disk is running out of its space.
What did you see instead? Under which circumstances?
The disk is running out of space.
System information
Linux 3.10.0-862.14.4.el7.x86_64 x86_64
Prometheus version
prometheus version 2.25

Prometheus configuration file
No response
Alertmanager version
No response
Alertmanager configuration file
No response
Logs
No response

@codesome what do you think about the proposal that's being made in this issue to add db.Close() in this particular location in the code?

It seems that Prometheus opened a head block but didn't close it.

That is how the TSDB works - head block stays open. The issue is that the compaction is not happening. If you are backfilling data (which is another way to look at this migration of data), it is required to allow overlapping blocks explicitly in older versions by design. In the newer versions since it is enabled by default, it is not a problem. So I consider this issue as a no-op.
Regarding the proposed db.Close(), no that would be wrong, we don't want to close the DB because a compaction failed somewhere. The resolution to this issue is to either enable overlapping blocks. Hence closing this issue.


Question / feature request: error handling / retries in Datastore

Question: What is current error handling procedure and retry approach? Documentation says

You can configure how many times API requests may be automatically retried. When an API request fails, the response will be inspected to see if the request meets criteria indicating that it may succeed on retry, such as 500 and 503 status codes or a specific internal error code such as rateLimitExceeded. If it meets the criteria, the request will be retried after a delay. If another error occurs, the delay will be increased before a subsequent attempt, until the retries limit is reached.

However from the code (service.rb, backoff.rb) I see that operation is only retried in case of grpc code 14. What is this code? It is not documented. Does it cover rate limit back off? What about network errors?
In the past with API v2 I used the following list of exception to retry the operation (in my wrapper):
    repeatable = exception.is_a?(Faraday::TimeoutError) || exception.is_a?(Faraday::ConnectionFailed)
    repeatable ||= exception.is_a?(Faraday::SSLError)
    repeatable ||= exception.try(:response).present? &&
                   exception.response.try(:env).present? &&
                   ((500..599).include?(exception.response.env.status) || [401, 409].include?(exception.response.env.status))
    repeatable ||= exception.is_a?(SocketError)
    repeatable ||= exception.is_a?(Errno::EPIPE) || exception.is_a?(Errno::ECONNRESET) || exception.is_a?(Errno::ECONNABORTED)

Which all have been observed during running on production and all proven to be retriable.
Feature request: Make retries configurable per request. There are different types of requests in application which require different handling of errors/retries. For example if error happening during web page load I'd better show error to user and let him retry rather than render page a minute later after background retries. Some operations are ok to fail as we can retry higher level application level transaction. But some operations are critical and must be retried heavily. So optional retries parameter to all API calls would be helpful.

@timanovsky Thank you for yet another well-written and thoughtful issue. Here are some supporting links: First, the retry handling for gRPC, which as you note, only tests for one error code: self.grpc_codes = [14] (which is UNAVAILABLE .)
The documentation that I could find for gRPC status codes does not contain the integer type codes, but you can see the code mappings in google/protobuf/stubs/status.h.

Here is the Datastore documentation for error codes.

It seems to me that retry handling (like a lot of things) should be customizable with sensible defaults. I agree that the smallest scope of customization should be a single method (API request), but it is also good to avoid duplication and support applying the same customization to multiple method calls. So I am wondering if an enclosing block might work.
This would be similar to the age-old pattern for transactions, so I'll include both in an example:
task_key = datastore.key "Task", "sampleTask"

datastore.retries limit: 10, codes: [:ABORTED, :DEADLINE_EXCEEDED, :UNAVAILABLE] do
  datastore.transaction do |tx|
    if tx.find(task_key).nil?
      task = datastore.entity task_key do |t|
        t["type"] = "Personal"
        t["done"] = false
        t["priority"] = 4
        t["description"] = "Learn Cloud Datastore"
      end
      tx.save task
    end
  end
end

Note that ABORTED, DEADLINE_EXCEEDED, and UNAVAILABLE are the three conditions in which retrying is recommended by the Datastore documentation, so they should probably be the defaults. But let's assume that the defaults are something different, and that this is an override.

In the future the retry behavior will be controlled these these GAX classes. I have no idea how (or if) gcloud-ruby exposes it. GAX should retry when needed, so I would tend towards ensuring that GAX behaves correctly over exposing the configuration in gcloud-ruby.

I would personally hope that the GAX configuration is accessible when using gcloud-ruby with GAX.
@blowmage Any timeline on moving to GAX? Would it be worth it for me to attempt a PR supporting per-method retry configuration for gRPC in the meantime?

I wouldn't want to add to the existing API before we know exactly what we can support across Google API Client, gRPC, and GAX.

To answer the original question, retries are now handled by Google API Client for REST services, and the code that used to be Gcloud::Backoff for gRPC services. But moving forward, retries for the gRPC services will handled by GAX. This change may happen as soon as the next major release.
The challenge is how gcloud-ruby maintains a consistent API across REST and gRPC services, across different dependencies. Right now that is the retries optional argument when creating new service objects.
require "gcloud"

gcloud = Gcloud.new "my-todo-project-id",
                    "/path/to/keyfile.json"
datastore = gcloud.datastore retries: 10

The onus is upon gcloud-ruby and its upstream dependencies to make sure that all errors that should be retried are retried. I think it was probably a mistake to expose Gcloud::Backoff in the public API.

@quartzmo Thanks for the link to documentation. From what I see there INTERNAL should also be retried. It is recommended once, but... Also it looks like RESOURCE_EXHAUSTEDis the quota exhaustion, then should not the library properly handle the back off?
And still not clear how network errors propagate (if they do) from grpc layer.
I have run the project on test setup for last 16 hours, but didn't see any error yet. It is not heavily loaded deployment though, with v2 API I saw about one error a day. And just for the record, and may be of interest to you, on heavily loaded production setup (v2) we see 1/10,000 error rate (all corrected by retries).

Here are more error codes: http://www.grpc.io/docs/guides/error.html

@timanovsky I'm very interested to hear what errors are raised in your fully-loaded deployment. Thanks again.

@quartzmo See code snippet in my initial comment. All cases were added after seeing on production (except that 500..599 range was added in full). You may wonder about seeing SSLError but yes, it is indeed happening, and it is indeed transient. Just sometimes SSL handshake is rejected by server.

@quartzmo Sorry, you probably meant new API. That I will be able to tell in a week probably

@timanovsky Yes, I meant the new one. And trust me, we want to do all we can to make sure you can continue to upgrade your production deployment(s). If you have any chance to look into GAX, and if you find anything in it that concerns you, please let us know.

@quartzmo  That's the first error surfaced :
Gcloud::InternalError: 13:{"created":"@1470385322.049611107","description":"OS Error","errno":32,"file":"src/core/lib/iomgr/tcp_posix.c","file_line":340,"os_error":"Broken pipe","syscall":"sendmsg"}
cause :  GRPC::BadStatus: 13:{"created":"@1470385322.049611107","description":"OS Error","errno":32,"file":"src/core/lib/iomgr/tcp_posix.c","file_line":340,"os_error":"Broken pipe","syscall":"sendmsg"}

Hmmmm, "Broken pipe"!
Any idea about this @blowmage?

No idea. It looks like a problem in the GRPC client though. What OS and hardware is this running on?
@murgatroid99 Do you have any input on this?

@blowmage This is on Heroku

FYI, i have seen this exception (broken pipe) three times so far. I believe it is some form of socket error.  For now I added retry logic in case of this exception.
THis one https://github.com/grpc/grpc/issues/7661 worries me much more though.

@timanovsky:

Is grpc/grpc#7661 possibly a gcloud-ruby bug?
Do you want to open a grpc issue for this broken pipe error?
Are you happy with the latest gcloud-ruby retry implementation? If so, are you ok to close this issue?

1 & 2 makes sense. Here we go: #817; https://github.com/grpc/grpc/issues/7663
3. How I can be happy if I need to keep observing my live deployment to see which exceptions pop up to application layer and then implement retry logic there. Sorry.

@timanovsky Makes sense. Can you suggest an API design for gcloud-ruby that will let you configure the retry implementation for arbitrary exceptions? The goal in my mind is that clients should never need to implement retry logic, but should have control over when retry occurs. (Do you agree?)

@quartzmo For me the retry API is a smaller problem than unknown/undocumented exceptions. The former is understood problem and if there is API in gclud-ruby I could use it, or I could use external retry logic. However not knowing what exceptions to expect and what returned exception classes really mean makes it very hard to build production grade system.
Nobody has answered yet what is throttling behavior. Are requests retried automatically at proper intervals, or I have to do it in upper layer. If later what exception code should I expect?

@timanovsky I agree with all points. We will do our best to improve the documentation of the possible exception types, as well as to document the retry intervals.

@timanovsky We are seeing the broken pipe errors several times a day on Heroku as well.
status=500 error='Gcloud::InternalError: 13:{"created":"@1471638568.698771416","description":"OS Error","errno":32,"file":"src/core/lib/iomgr/tcp_posix.c","file_line":340,"os_error":"Broken pipe","syscall":"sendmsg"}'

Are you finding an application retry successful?

yes, retrying has always been successful.
Best regards,
Alexey Timanovsky.
On 22 August 2016 at 19:49, Bryce McLean<EMAIL_ADDRESS>wrote:

@timanovsky https://github.com/timanovsky We are seeing the broken pipe
errors several times a day on Heroku as well.
status=500 error='Gcloud::InternalError: 13:{"created":"@1471638568.698771416","description":"OS Error","errno":32,"file":"src/core/lib/iomgr/tcp_posix.c","file_line":340,"os_error":"Broken pipe","syscall":"sendmsg"}'
Are you finding an application retry successful?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/GoogleCloudPlatform/gcloud-ruby/issues/808#issuecomment-241475858,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABK0FggCl2rsmdfv6YYaVM3Ce-hmf2iqks5qidM3gaJpZM4JV5ZF
.

I believe this has been resolved by exposing the GAPIC client configuration. But please feel free to reopen if you still encounter issues with retry.


Consider moving to outline-focused single-cell segmentation displays instead of masks

This is the exact example I was referencing! That 568 row looks like it should be split into two cells, which I think is the issue with using masks over outlines.

Originally posted by @jenna-tomkinson in https://github.com/WayScience/CytoDataFrame/pull/6#discussion_r1821500425

Addressed as isolated way of viewing segmentation in #18 . We decided to keep masks and outlines as distinct ways of showing segments.


feat: port Archive.100-theorems-list.42InverseTriangleSum

[ ] depends on: leanprover-community/mathlib#19195
this took <10s

It looks like Lean 4 doesn't understand import Archive.100-theorems-list.42InverseTriangleSum

I've created the topic.
https://leanprover.zulipchat.com/#narrow/stream/287929-mathlib4/topic/Archive.2E100-theorems-list.2EXXTheoremName.20causes.20error.20!4.235114

This PR/issue depends on:

leanprover-community/mathlib#19195
By Dependent Issues (🤖). Happy coding!

This PR/issue depends on:

leanprover-community/mathlib#19195
By Dependent Issues (🤖). Happy coding!

Might be easier to make a new branch

mathlib3port is updated.


feat(stryker config): rename config setting timeoutMs to timeoutMS

It should solve Rename 'timeoutMs' config key to 'timeoutMS'

@simondel #1076 is also done but without tests. I dunno how to make them. Should I add it here without tests / create new pull request without tests or leave it to you?

@kmdrGroch Nice! note: the build may still fail because you removed the " in your test but it's still in the line of code that logs the warning

@simondel that's why copy-paste is bad >.< final try

Awesome job!
Please open a new pull request for #1076. I'll help you with the tests 👍


Fixed link references to Code of Conduct file

Description / Motivation
The current links to the Code of Conduct file in GitHub are broken and show a 404.
How Has This Been Tested?
In VS Code markdown Preview and on Vercel preview site
Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[X] Documentation update (non-breaking change; modified files are limited to the /data directory or other markdown files)

Checklist:

[X] I have read the Contributing guide.
[X] My code/comments/docs fully adhere to the Code of Conduct.
[ ] My change is a code change.
[X] My change is a documentation change and there are NO other updates required.
[ ] My change has new or updated images which are stored in the /public/images folder that need to be migrated to Sitecore DAM

I am force merging this as it is simple MD readme files, no impact to the site.


Rename "web"  to "frontend"

I hope I got it right, though additional check from you will be helpful.
Assets still hosted with http://{host}/web prefix

Fixed Dockerfile.artifacts


UnsupportedPythonVersion with two Pipfile [[source]] blocks

When installing python-swiftclient from a Pipfile with a single [[source]] block, like so:
[[source]]
url = "https://pypi.python.org/simple"
verify_ssl = true
name = "pypi"

[packages]
python-swiftclient = {version="==3.4.0", index="pypi"}

python-swiftclient installs successfully. So far so good. But, when my Pipfile has two valid [[source]] blocks...
[[source]]
url = "https://pypi.python.org/simple"
verify_ssl = true
name = "pypi"

[[source]]
url = "https://pypi.companyname.com"
verify_ssl = true
name = "privatepypi"

[packages]
python-swiftclient = {version="==3.4.0", index="pypi"}

Installation of python-swiftclient errors with...
pip.exceptions.UnsupportedPythonVersion: futures requires
Python '>=2.6, <3' but the running Python is 3.6.3

...despite explicitly specifying which index python-swiftclient should be installed from (index="pypi"). Why does having a second (and theoretically unused) [[source]] block affect installation of that package? (Of note, privatepypi does not contain a fork of python-swiftclient.)
Describe your environment

OS Type: macOS Sierra 10.12.6
Python version: 3.6.3
Pipenv version: 8.3.2

Expected result
I expected python-swiftclient to install from public pypi without issue when two [[source]] blocks are present and the correct index to use is specified for the given package.
Actual result
python-swiftclient fails to install when two [[source]] blocks are present, suggesting that the un-referenced [[source]] is being consulted or is affecting dependency resolution somehow.
Steps to replicate
Write this to a Pipfile:
[[source]]
url = "https://pypi.python.org/simple"
verify_ssl = true
name = "pypi"

[[source]]
url = "https://pypi.companyname.com"
verify_ssl = true
name = "privatepypi"

[packages]
python-swiftclient = {version="==3.4.0", index="pypi"}

Then do pipenv --three install to observe the error:
pip.exceptions.UnsupportedPythonVersion: futures
requires Python '>=2.6, <3' but the running Python is 3.6.3

Any suggestions? Thanks!

@commandtab I actually can't reproduce this with your pipfile which means one of a few things:

You may have pipenv installed via pip on python 2,7, which could be causing a problem here.  I'd recommend installing pipenv via pip3 and using that
You may be running into a caching issue, which could be resolved via pipenv lock --clear

Having played around a tiny bit with this I think the first case is the most likely one, we have a known (and complicated) dependency resolution issue when crossing from python 2.7 installed pipenv into python 3. Can you give us the output of pipenv lock --verbose --clear before you reinstall pipenv with pip3?
Thanks for the report!
/cc @vphilippon

Thanks so much! You're right on with 1!
I have Python 2 and 3 installed via Homebrew because I maintain projects written against both versions, and pipenv is installed under Python 2.7.14 (it was the default pip when doing pip install pipenv). However, this particular project in question is a Python 3.6.3 one. Here is the output of pipenv lock --verbose --clear under this arrangement:
https://gist.github.com/commandtab/9b88410e02aff38fd8b70d5bd2ba7b92
After uninstalling pipenv using Python 2's pip and reinstalling it with Python 3's pip, the same command successfully installs python-swiftclient 🎉   Here's the output for that, just in case:
https://gist.github.com/commandtab/cfbc1db2956c26dd92492a3d8f08499e
Now part of me is wondering if the reverse will run into a similar problem, where pipenv is installed in a 3.x environment and is installing packages for a 2.7.x project... 😅
Thanks again for your help! I truly appreciate it.

@commandtab sure thing, we see many more issues going from python 2 installed pipenv to python 3 environments than the other way around.  Pipenv on python 3 works pretty consistently in my own experience.
As a sidenote after I commented yesterday I did manage to run the same script and install a python 3 environment using a python 2 installation of pipenv via pyenv using pipenv 9.0, but I will note that it failed once and took quite awhile.  I'll close this for now since we are tracking this in a few places but I'll make note of the lock output since that is what is relevant to solving the dependency issue.  Thanks for helping us troubleshoot!


Allowing the use of tokio's AsyncBufRead

Supporting async buffer streams is a major missing feature I think. In the past there has been quite a bit of discussion about it and it looks even like it has mostly been finished. It might even just be waiting for a merge. Would be great if this can be rescued
https://github.com/tafia/quick-xml/pull/314

PR is welcome! As noticed in the tafia/quick-xml#314, I would prefer to reuse a existing code rather than duplicate it -- or using macros, or introduce a new traits / methods in the existing traits


[bugFix][ui]Click Cancel, the node data is restored to the original data

Codecov Report

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

@@          Coverage Diff           @@
##             dev    #3255   +/-   ##
======================================
  Coverage       ?   34.14%
  Complexity     ?     2421
======================================
  Files          ?      443
  Lines          ?    20637
  Branches       ?     2531
======================================
  Hits           ?     7047
  Misses         ?    12928
  Partials       ?      662

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 a03d548...edf1115. Read the comment docs.


Output root-modules with only modules that are not sourced

A user could have a monorepo full of sibling modules. Or, they could have a root module and then modules/child called in the root module. In this case, modules will return both the root and the child, but root-modules would only return the root. Some tools will want to run on all modules (e.g., TFLint) while others should only run on roots (terraform validate).
Determining root modules requires parsing the configuration to find the source value for module calls. That will probably require https://github.com/hashicorp/terraform-config-inspect and may justify converting this action to Go.

#173 indirectly addresses this use case. If you conventionally put modules in ./modules directories, you can customize the glob to just match those.


quality control error

In the data evaluation and the evaluation of the quality is an error in the source code. The data are taken correctly on botometers but the evaluation is pushed incorrectly into the database.

import time
import models
from utils import *
def get_all_relationships(screen_name):
subject = models.Subject.get(models.Subject.screen_name == screen_name)
relationships = models.Relationship.select().where(
models.Relationship.subject == subject)
return relationships
def get_subject_follower_quality(screen_name):
logger.info('Checking Quality for subject: {}'.format(screen_name))
chunk = 1 / 6 ---> 1.66666666 <--- is working
quality = {}
quality['no_timeline'] = 0
quality['protected'] = 0
quality['very_good'] = 0
quality['good'] = 0
quality['neutral'] = 0
quality['poor'] = 0
quality['very_poor'] = 0
quality['total'] = 0

I don't understand this issue. Could you please try to reexplain it.

The problem is that the operation chunk = 1/6 is not executed properly and therefore every user is very_poor. I change 1/6 to 0.1666 .. the code works

This is a Python 2 vs Python 3 issue related to PEP 238.
Python 2.7.12 (default, Dec  4 2017, 14:50:18)
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> a = 1 / 6
>>> a
0

Python 3.5.2 (default, Nov 23 2017, 16:37:01)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> a = 1 / 6
>>> a
0.16666666666666666

Python 2 is reaching end of life and is not supported by this project.


MQTT5 - Support Unsubscribe action.

Support unsubscribe action. all action will response acknowledgement reason code.

Success.
No subscription existed.
Topic Filter invalid.
Unspecified error.

@Technoboy-  please take a review.

I need to reformat the code..

@Technoboy-
All things have been fixed, PTAL : )


chore(main): release 0.6.9

:robot: I have created a release beep boop
0.6.9 (2023-11-10)
Bug Fixes

deps: update module golang.org/x/net to v0.18.0 (#33) (5d1f3c3)

This PR was generated with Release Please. See documentation.

:robot: Release is at https://github.com/mikesmitty/mdns-mesh/releases/tag/v0.6.9 :sunflower:


Bolt publish fails because npm can give wrong information quite often

Through no fault of this project, bolt publish can still fail when npm has data replication issues.
We are now seeing this issue tens of times a day in the atlaskit-mk-2 repo.
Scenario

A package is published (e.g<EMAIL_ADDRESS>Later than day bolt publish is run to release any unrelased packages.

bolt runs npm info for each package in the repo to find which are ahead of npm
npm says that the latest version of pkg-a is 1.0.0

local version is 1.0.1 so bolt marks it as ready to release

bolt attempts to publish<EMAIL_ADDRESS>npm returns a 403 as you cannot published over an already published version.

Suggested fixes

We could look for another source of truth for which versions have been deployed. Unfortunately:

npm info can be wrong
yarn info can be wrong
unpkg can be wrong (but is correct more often than npm info)

🚫So this likely wouldn't solve the problem (unless there is a better source of truth)

We could look at the error messages that come back from npm and ignore them if they are 403's with the message saying you cant publish over an existing version

✅ moderately safe, since we know there must be a mistake on npm side since we must have just run and npm info which gave us conflicting info
🚫 feels slightly risky as it's ignoring what could be a legitimate error (race condition in a package being released in two places at once).

Not possible in atlaskit, but possible in other places?

We could change the way publish works and take an explicit list of packages to publish and ignore the others

🚫 This would remove some of our auto-recoverability which would need to be solved another way
🚫 Would be a breaking change to current

Personally, I'd lean towards option 2. Happy to put it behind a flag if necessary though.
Thoughts?

CC @jamiebuilds


UploadedFile::fake() error

It will give this error when using UploadedFile::fake()->image('avatar.png') in testing
testing.ERROR: Call to undefined function Intervention\Image\Gd\imagejpeg()

Hi, thanks for sharing. I'm fairly confident this is because of #21.
I'll mark that as a duplicate in order to keep all information in one thread.
Duplicate of #21


Add LICENSE

This repo is missing a LICENSE file.

Added, MIT license was already specified in Cargo.toml


Question about Teensy and STM32

Hi, great project, nice that it is open source. I have tried to compile it on platformio for Arduino Leonardo, and this compiles fine. But when I try to compile for Teensy 3.1, it fails with a  PluggableUSB.h: No such file or directory
error. Have you tried it on a Teensy? And what STM32 board are you using?

Currently it is not support teensy. Maybe i will try in the future.
I am using stm32f407.


Add "Configure your authorized organizations" message to My Tools page

Description
This PR adds the "Configure your authorized organizations" message to the My Tools page so it matches the My Workflows page.
Review Instructions
Verify that the My Tools page has the same "Configure your authorized organizations" as the My Workflows page in the left side bar.
Click on the "Configure" link and verify that it takes you to configure the app for the correct environment. For example, if you're in QA, then it should display the GitHub OAuth app for QA.
Issue
dockstore/dockstore#4852
https://ucsc-cgl.atlassian.net/browse/DOCK-2136
Security
If there are any concerns that require extra attention from the security team, highlight them here.
Please make sure that you've checked the following before submitting your pull request. Thanks!

[x] Check that your code compiles by running npm run build
[x] Ensure that the PR targets the correct branch. Check the milestone or fix version of the ticket.
[x] If this is the first time you're submitting a PR or even if you just need a refresher, consider reviewing our style guide
[x] Do not bypass Angular sanitization (bypassSecurityTrustHtml, etc.), or justify why you need to do so
[x] If displaying markdown, use the markdown-wrapper component, which does extra sanitization
[x] Do not use cookies, although this may change in the future
[x] Run npm audit and ensure you are not introducing new vulnerabilities
[x] Do due diligence on new 3rd party libraries, checking for CVEs
[x] Don't allow user-uploaded images to be served from the Dockstore domain
[x] If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.
[x] Check whether this PR disables tests. If it legitimately needs to disable a test, create a new ticket to re-enable it in a specific milestone.

See discussion in https://github.com/dockstore/dockstore-ui2/pull/1988#discussion_r1670867271
I changed the "Configure" link so that it uses the GitHub client ID from the configuration file and redirects you to the settings page for the environment's OAuth app. Previously, it always sent you to the settings page of prod's OAuth app because the link was hardcoded.
Re-requesting review


2.1 release note

Signed-off-by: bowenlan-amzn<EMAIL_ADDRESS>Issue #, if available:
Description of changes:
CheckList:

[x] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Codecov Report

Merging #405 (96c5658) into main (fd06db7) will decrease coverage by 0.12%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main     #405      +/-   ##
============================================
- Coverage     75.68%   75.56%   -0.13%
+ Complexity     2460     2453       -7
============================================
  Files           313      313
  Lines         14426    14426
  Branches       2230     2230
============================================
- Hits          10919    10901      -18
- Misses         2263     2277      +14
- Partials       1244     1248       +4

Impacted Files
Coverage Δ

...nt/indexstatemanagement/model/destination/Chime.kt
55.55% <0.00%> (-22.23%)
:arrow_down:

...ensearch/indexmanagement/util/JobSchedulerUtils.kt
73.91% <0.00%> (-8.70%)
:arrow_down:

...ansform/action/explain/ExplainTransformResponse.kt
79.31% <0.00%> (-6.90%)
:arrow_down:

...gement/indexstatemanagement/action/ShrinkAction.kt
64.70% <0.00%> (-5.89%)
:arrow_down:

.../rollup/action/start/TransportStartRollupAction.kt
72.94% <0.00%> (-2.36%)
:arrow_down:

...management/indexstatemanagement/MetadataService.kt
62.68% <0.00%> (-2.24%)
:arrow_down:

...nt/indexstatemanagement/ManagedIndexCoordinator.kt
68.71% <0.00%> (-1.17%)
:arrow_down:

.../opensearch/indexmanagement/rollup/model/Rollup.kt
85.58% <0.00%> (-0.94%)
:arrow_down:

...earch/indexmanagement/transform/model/Transform.kt
84.61% <0.00%> (-0.86%)
:arrow_down:

...nsform/action/stop/TransportStopTransformAction.kt
77.77% <0.00%> (ø)

... and 2 more

Continue to review full report at Codecov.

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


one page two scroller

a large container,has a scrollerY,
and the large container has an inner small container,the small has a scrollerY too,
then the small container scroller not work

I am looking at this feature to.
The thing is I want to achieve a lazy loading effect for articles wherein I am able to scroll from article to another. I have to lazy load the previous and next article while viewing the current article.


Custom model for Colab?

I managed to get the Colab running but I found out that the base model is currently restricted to:
runwayml/stable-diffusion-v1-5
Is there a way that we can use a custom model for the standalone Colab?
Otherwise, I would have to load up A1111 instead.

@edwinzeng2005 hi, if you want to use a custom model for Colab, you should use a model with diffusers format. you can use script (https://github.com/huggingface/diffusers/blob/main/scripts/convert_original_stable_diffusion_to_diffusers.py) to convert a .ckpt file to diffusers


refactor(state_table): unify iterator interfaces of state table

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Currently behaviors of executors scanning state table are as follows:

iter_row: dynamic_filter, now
iter_row_with_pk_prefix: join, agg_min, eowc, over_window, top_n
iter_row_with_pk_range: dynamic_filter, sort_buffer, backfill, source

As https://github.com/risingwavelabs/risingwave/pull/12251 introduced a new interface iter_row_with_pk_prefix_sub_range, iter_row_with_pk_prefix can be replaced by it, as well as iter_row.
This PR aims to unify the upper-level executors calls to the state table iterator interface and eliminate some ambiguities.
New interfaces:

iter_row
prefix_iter_row: iter row with given prefix(without vnode), and sub_range is needed. When sub_range is (Ubounded, Unbounded), it will scan all rows under this prefix.
vnode_iter_row: : iter row with given vnode and pk range.

Checklist

[ ] I have written necessary rustdoc comments
[ ] I have added necessary unit tests and integration tests
[ ] I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features #7934).
[ ] My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
[x] All checks passed in ./risedev check (or alias, ./risedev c)
[ ] My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

[ ] My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

[ ] My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

@hzxa21 PTAL❤


New helper to stay under rate-limits policies of consumed third-parties.

It also provides docido_sdk.toolbox.loader.resolve_name that allows
you load referenced Python code in YAML configuration very easily.

Coverage increased (+0.2%) to 92.894% when pulling 483a52d881558044a1dbfa87ca9cdf446f4382a9 on feat/rate_limits into 9fb184344d1a4a5e0bc79fcef9a742700ca20f25 on master.

@tuxnco unit-tests are a little bit cumbersome but take a look at test_rate_limiter.yml that shows you how rate-limit policies can be described.

ping @tuxnco can you please review

@tristan0x sorry for the delay, here is my review:
The proposed design is beautiful and powerful. From my understanding, it allows us to cover a broad variety of use cases:

Contextual counters / limiters
Stacked limiters (e.g. amount of qps per IP + overall QPS per day or whatever), which is brilliant

I have a few concerns, though:

blocking mode: if waits=True, return of __call__ should be ignored (by design, or could be None ?) May be the wait mode should just be a wrapper around __call__ performing the wait, no return value expected.
RateLimiter: I can't figure out how race conditions are prevented : if several threads call a given instance, we might end up having self.count > self.__calls_per_window, which is not desired.
RateLimiter: Having the window_size expressed in seconds complicates everything. Maybe configured window size could be expressed in seconds, the internal window and __window_size expressed in milliseconds, and eax expressed in seconds. So that user oriented numbers are human readable / understandable, and amounts involved in computations are in the same unit, avoiding * 1e3 / 1e3 here and there.
RateLimiterPersistence : I have a concern with the proposed URI construction. This could simply be callable (with your impl as a default). For instance, a user using columns in context's keys could end up sharing his counter with someone else.
RateLimiterPersistence: from my point of view, the persistence can not be handled the way it is actually : we save unconditionnally, like if the given instance is the only one with write access to the persistence layer. I think the rate limiter should be loaded at each call, and save operations should check for concurrent modification, and potentially reject the call, or retry by reloading / merging state. Things can get complicated here, not sure that's the best place to handle this.
RateLimiterPersistence: a key retention or expiration mechanism should be available. Ideally we can bind that to the store used to handle the persistence (Redis has expiring keys, MongoDB has TTL indexes, ...)

What do you think of this : http://redis.io/commands/INCR#pattern-rate-limiter-1

If I understand well, each RateLimiter instance has a single bucket. Would it make sense to have RateLimiters for multiple buckets for your use case ?
For instance all twitter API points that have the same rate limit could use the same RateLimiter where each bucket is <user> + <end point>. And each bucket has its own windows of course.
This way you can keep the number of instances of RateLimiter low when you have a large number of <user> + <end point>. The storage and persistence can be optimized as well.
I'm just asking if it's interesting here or not.

Thank you Tanguy for your comment.

You are absolutely right about the race conditions here. This first version of this rate limiter is client-side and only works in a single-thread context. The only persistence layer available to crawlers is thru the get_kv set_kv methods IndexAPI, which does not not provide consistency. So, for now, we are screwed.
I didn't want to save the context at each call to __call__ in the first place. But then, I realised number of calls made is not something you can to forget in case something goes bad afterward. Your programs made a certain amount of API calls, whether your process will raises an exception later or not.
I agree we need to provide consist time units to the customer. I will look after that.
I agree the returned value of __call__(waits=True) is purely informative, and tells the caller how long it blocks in the method. Do you think if can be confusive?
Regarding the key computation, I think all the information should remain in the YAML configuration. If the hash method is the customer's responsibility, you can be sure there will be conflicts somehow. If you think of the rate-limiter as a service, the hash method is definitely private. It is the responsibility of the API that there won't have any conflict between 2 customers.

I already submitted the link you share in the TellMeWhen technical specification page. So I could not agree more :)

@phildav What you mean it to provide a context used to build the key when using a RateLimiter instance, in __call__ ?
For instance:
YAML configuration:
rate_limits:
  twitter:
    persistence: docido_sdk.toolbox.rate_limits.IndexAPIRLPersistence
    window_size_sec: 1
    calls_per_window: 5
    key:
        - docido
        - twitter
        - "{user}"
        - "{stream_id}"

And then use it this way:
api = get_twitter_api(user='john.doe')
rate_limiter = get_rate_limiter("twitter")

rate_limiter(stream_id=4242)
api.read_stream(id=4242)

rate_limiter(stream_id=4243)
api.read_stream(id=4242)

If so I think it is brilliant !

Coverage increased (+0.2%) to 92.894% when pulling b7de1c90afaeae7a6663c828bed4a8ac97f280f0 on feat/rate_limits into 9fb184344d1a4a5e0bc79fcef9a742700ca20f25 on master.

Coverage increased (+0.1%) to 92.885% when pulling 5057e7245efff0eab70baeebd93cf7fca4857267 on feat/rate_limits into 9fb184344d1a4a5e0bc79fcef9a742700ca20f25 on master.

Coverage decreased (-0.1%) to 92.99% when pulling 45199e6019d1239cabb2bf955f5d65160bf37392 on feat/rate_limits into 471eb97752de783709f374310bf5314a5994f4e0 on master.

Coverage increased (+0.5%) to 93.574% when pulling a3b6e0c781d1ea7922a288dee9a703932b2af606 on feat/rate_limits into 101a86a3da7d8267c6160779734e10fe54bd255c on master.

Coverage increased (+0.4%) to 93.487% when pulling c4947c94fa403a2b42df17fbc8715d90317134d2 on feat/rate_limits into 101a86a3da7d8267c6160779734e10fe54bd255c on master.

Coverage increased (+0.4%) to 93.503% when pulling b2c15f5d4232b67d7820bf94574bf8d07d0d536f on feat/rate_limits into 101a86a3da7d8267c6160779734e10fe54bd255c on master.


Correct date format for Serbian Wikipedia

Please make date format for date, access[-]date and archive[-]date parameters in cite templates be (d)d. [m]m. yyyy instead of dd.mm.yyyy (for e.g. it's correct 13. 7. 2019 only; other ways of writing it like 13.7.2019 or 13.7. 2019 or 13. 07. 2019 are incorrect). Shortly said, the problem is that space is missing between day and month number and month and year number.

According to https://en.wikipedia.org/wiki/Date_and_time_notation_in_Serbia "the space is, however, optional, as dates could be written and without the space (for example 31.12.2006.)."


CAMEL-10666: exposed uri param to enable inclusion of serializable headers

As per CAMEL-10666 this is an enhancement proposal to restore the possibility of saving Serializable headers within the exchange, simply setting to true the new parameter allowSerializedHeaders.
Please note this pull request is aimed to replace and obsolete https://github.com/apache/camel/pull/1370, in order to overcome issues on adding new options to minor releases (as requested by @oscerd).

The PR has been merged. Thanks @davidecavestro, can you close this?


Red Link in hagure no yuusha Aesthetica

Hello i had such many trouble .. Many chapter in some novels had red link and when i open it.. It say empty content ... Please i need help

Can you open the BakaTsuki website from your phone?
I can open it in my device (Android 5.1)

Umm sorry i mean it was about bakareader app.. I try this app for 3 days and when i check some novels it had many missing chapters like hagure yuusha no aesthetica volume 2 completely missing .. I can open bakatsuki website use my browser.. and i check hagure no yuusha use browser .. And i found volume 2 no text too... Did Hagure Yuusha novels chapt

Just to clarify with you: so all the available chapters in baka tsuki website can be loaded in the app?

And i found volume 2 no text and it was deleted... Did hagure yuusha novels not translated completely ? Because volume 4 in bakareader is avalaiable... And only volume 2 and 3 are missing

I have no idea, you better check with the translator on bakatsuki forum.

Umm yeah i think i not check it properly... The other volume are avalaiable...

Okay thanks for the help


Deleting entities gives too few arguments error

When calling delete() from Contact or Webhook entity it produces an error:
Too few arguments to function Picqer\Financials\Moneybird\Connection::delete()
In Picqer\Financials\Moneybird\Actions\Removable line 18:
return $this->connection()->delete($this->getEndpoint() . '/' . urlencode($this->id));
Delete is called without the second $body parameter which produces this error.

Excuse me, wrong library!


Fix core-command response when handling a CBOR result.

This is the second (thought first in sequence of being reviewed) of two PRs that will address https://github.com/edgexfoundry/edgex-go/issues/2272 ("Response of issueDeviceCommand is not providing CBOR Content-Type header").
The second PR (in device-sdk-go) was previously submitted, with details on sequencing here.
Blackbox Integration tests were run, to validate the updates per this PR (Results are provided in the comment below).
The set of steps used for testing are provided in the comment below.
After submitting this PR, I realize that we would be better served by marking this as a Draft PR (and I'll mark this as a Draft PR once the Jenkins build checks finish running, at which time it should again become possible to do so) - The thinking here is that as we work through the best approach to resolving a challenge that was encountered during the work done.
The insight referred to above is thanks entirely to @tsconn23: The handling of PUT and GET is done significantly down a deeply nested call to functions (as follows) and we may be better served by making them (i.e. PUT and GET) first class citizens by creating a type abstraction for them, which can be tapped into at a much higher level in the call chain, enabling the request and client to be assembled higher up the chain:

github.com/edgexfoundry/edgex-go/internal/core/command.serviceCommand.Execute at types.go:47
github.com/edgexfoundry/edgex-go/internal/core/command.executeCommandByDevice at device.go:128
github.com/edgexfoundry/edgex-go/internal/core/command.executeCommandByDeviceID at device.go:72
github.com/edgexfoundry/edgex-go/internal/core/command.issueDeviceCommand at restDevice.go:79
github.com/edgexfoundry/edgex-go/internal/core/command.restGetDeviceCommandByCommandID at restDevice.go:42
github.com/edgexfoundry/edgex-go/internal/core/command.TestRestGetDeviceCommandByCommandID.func1 at restDevice_test.go:272

Sample results for Blackbox Integration tests are as follows:
Issued the following command for the registered device:

curl --location --request GET 'http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192'

And got this response as expected:

{
    "id": "9f872d68-2281-4af4-959d-29e4d51c2192",
    "name": "Random-Binary-Device",
    "adminState": "UNLOCKED",
    "operatingState": "ENABLED",
    "labels": [
        "device-virtual-example"
    ],
    "commands": [
        {
            "created":<PHONE_NUMBER>178,
            "modified":<PHONE_NUMBER>178,
            "id": "b349df4a-6c3d-4218-b8bc-7ff8d51ea50d",
            "name": "Binary",
            "get": {
                "path": "/api/v1/device/{deviceId}/Binary",
                "responses": [
                    {
                        "code": "200",
                        "expectedValues": [
                            "Binary"
                        ]
                    },
                    {
                        "code": "503",
                        "description": "service unavailable"
                    }
                ],
                "url": "http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192/command/b349df4a-6c3d-4218-b8bc-7ff8d51ea50d"
            },
            "put": {
                "path": "/api/v1/device/{deviceId}/Binary",
                "responses": [
                    {
                        "code": "200"
                    },
                    {
                        "code": "503",
                        "description": "service unavailable"
                    }
                ],
                "url": "http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192/command/b349df4a-6c3d-4218-b8bc-7ff8d51ea50d",
                "parameterNames": [
                    "EnableRandomization_Binary"
                ]
            }
        }
    ]
}

Here are the set of steps used for DEV testing:

Launch all edgex-go core and support services locally, running natively, without containerization (This will be needed for the next step).
Locally build this device-sdk-go branch.
Point device-virtual-go to use this device-sdk-go branch (i.e. the local build) and launch device-virtual-go locally.
At this point, the virtual device named "Random-Binary-Device" will get registered with EdegeX.
Issue the following command:

curl --location --request GET 'http://localhost:48082/api/v1/device/name/Random-Binary-Device' \
--header 'Content-Type: application/cbor' \
--data-raw ''

The preceding command should respond with two URLs, one each to use for PUT and GET, for example as follows:

{
   "id":"9f872d68-2281-4af4-959d-29e4d51c2192",
   "name":"Random-Binary-Device",
   "adminState":"UNLOCKED",
   "operatingState":"ENABLED",
   "labels":[
      "device-virtual-example"
   ],
   "commands":[
      {
         "created":1583181972178,
         "modified":1583181972178,
         "id":"b349df4a-6c3d-4218-b8bc-7ff8d51ea50d",
         "name":"Binary",
         "get":{
            "path":"/api/v1/device/{deviceId}/Binary",
            "responses":[
               {
                  "code":"200",
                  "expectedValues":[
                     "Binary"
                  ]
               },
               {
                  "code":"503",
                  "description":"service unavailable"
               }
            ],
            "url":"http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192/command/b349df4a-6c3d-4218-b8bc-7ff8d51ea50d"
         },
         "put":{
            "path":"/api/v1/device/{deviceId}/Binary",
            "responses":[
               {
                  "code":"200"
               },
               {
                  "code":"503",
                  "description":"service unavailable"
               }
            ],
            "url":"http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192/command/b349df4a-6c3d-4218-b8bc-7ff8d51ea50d",
            "parameterNames":[
               "EnableRandomization_Binary"
            ]
         }
      }
   ]
}

Use the URL for PUT from above response as follows, where the --data-binary is supplied via a local file, whose contents are °xEnableRandomization_Binarydtrue, so you'll want to replace the path '@/Users/username/Decoded_Binary' by a suitably-located local file on your filesystem:

curl --location --request PUT 'http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192/command/b349df4a-6c3d-4218-b8bc-7ff8d51ea50d' \
--header 'Content-Type: application/cbor' \
--data-binary '@/Users/username/Decoded_Binary'

And use the URL for GET from the same response  above as follows:

curl --location --request GET 'http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192/command/b349df4a-6c3d-4218-b8bc-7ff8d51ea50d' \
--header 'Content-Type: application/cbor'

Codecov Report

Merging #2446 into master will increase coverage by 0.65%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #2446      +/-   ##
==========================================
+ Coverage   37.57%   38.23%   +0.65%
==========================================
  Files         157      157
  Lines       13176    12851     -325
==========================================
- Hits         4951     4913      -38
+ Misses       7962     7670     -292
- Partials      263      268       +5

Impacted Files
Coverage Δ

internal/core/command/router.go
0.00% <0.00%> (ø)

internal/core/command/restDevice.go
34.28% <50.00%> (+34.28%)
:arrow_up:

internal/core/command/device.go
50.00% <71.42%> (+16.17%)
:arrow_up:

internal/core/command/put.go
85.71% <100.00%> (+7.93%)
:arrow_up:

internal/core/metadata/rest_deviceprofile.go
90.76% <0.00%> (-0.92%)
:arrow_down:

internal/core/metadata/router.go
0.00% <0.00%> (ø)

internal/core/metadata/rest_provisionwatcher.go
0.00% <0.00%> (ø)

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 4f3c71d...f39f9f2. Read the comment docs.

Rebased

Rebased

@akramtexas, I forgot to mention one caveat regarding the reduction of arguments, with the body argument I think you can only read the contents of the request body one time. If that is the case then reading the request body and passing it as arguments alongside the *http.Request might be desirable. Please verify that is the case before reducing the arguments.

@akramtexas, I forgot to mention one caveat regarding the reduction of arguments, with the body argument I think you can only read the contents of the request body one time. If that is the case then reading the request body and passing it as arguments alongside the *http.Request might be desirable. Please verify that is the case before reducing the arguments.

@AnthonyMBonafide Thanks, appreciated. Will do 👍

@akramtexas, I forgot to mention one caveat regarding the reduction of arguments, with the body argument I think you can only read the contents of the request body one time. If that is the case then reading the request body and passing it as arguments alongside the *http.Request might be desirable. Please verify that is the case before reducing the arguments.

@AnthonyMBonafide Thanks, appreciated. Will do 👍

@AnthonyMBonafide What I found was that indeed, one can only read the contents of the request body one time (So reading the request body and passing it as an argument alongside the *http.Request is the thing to do). Plus, attempting to to read the request body earlier in the deeply nested call chain, in func NewPutCommand(...) {...} introduces its own problems, such as introducing error-handling at a much-lower-level than desirable, for example:

	if r != nil {
		body, err := ioutil.ReadAll(r.Body)
		if body == nil && err != nil {
			httpErrorHandler.Handle(w, err, errorconcept.Common.InvalidRequest_StatusBadRequest)
			return "", err
		}
	}

In summary, reading the request body and passing it as an argument alongside the *http.Request is the thing to do 👍

Rebased, plus addressed nearly all comments. Will request review after remaining comments have also been addressed.

Rebased.

Rebased

Rebased

Recheck

Noticed an error, unrelated to this PR, in the Jenkins build:

[2020-04-02T22:50:09.777Z] go: finding gopkg.in/yaml.v2 v2.2.8
[2020-04-02T22:50:09.777Z] build github.com/edgexfoundry/edgex-go/cmd/config-seed: cannot load golang_org/x/crypto/chacha20poly1305: malformed module path "golang_org/x/crypto/chacha20poly1305": missing dot in first path element
[2020-04-02T22:50:09.777Z] Makefile:74: recipe for target 'test' failed
[2020-04-02T22:50:09.777Z] make: *** [test] Error 1
script returned exit code 2

Rebased

@AnthonyMBonafide Based on the helpful discussion earlier today, it makes sense to look at the two flows you outlined as follows:
Postman ----> Request  ----> core-command ----> Request  ----> device-sdk-go
Postman <---- Response <---- core-command <---- Response <---- device-sdk-go

In relooking the flows, I think we have all the information we need to set the returned header Content-type properly, based on the header Content-type that is received in the Device Service request. The content itself (CBOR or JSON) is (and was) being returned properly, and it was only the header Content-type that was hard-coded to JSON, and which is fixed in this push.
I have not squashed the commits so that it remains easy to diff the changes going forward, at the same time giving the flexibility to revert a change.

Following a realignment in the approach (as related to the associated PR in device-sdk-go), the work underway in that PR (i.e. in device-sdk-go) will be completed first, after which this current PR will be brought to closure.
Meanwhile, pushing to the relevant branch (in edgex-go) the work completed for addressing how we need to get the headers from the response from the response of the device service, which, at this point, only exists in the underlying operator (as in responseBody, responseCode, err := ex.Execute()).

@akramtexas Sorry if I missed the messaging in standup, but is this ready for review or does it need work done in the device services repo first?

@akramtexas Sorry if I missed the messaging in standup, but is this ready for review or does it need work done in the device services repo first?

@brandonforster Yes, this is ready for review, thanks for checking 👍 This PR does not depend on the work done in device-sdk-go, which will be handled separately, at a later time, per the following: https://github.com/edgexfoundry/device-sdk-go/issues/488

Rebased

Completed the functional testing (after addressing how we should not only read the device service response body the first time, but also propagate it at that time, because the response body can evidently be read only once, during the first time only).
I rebased so as to pull in Daniel's PR (Swap ugorji/go with fxamacker/cbor library) at https://github.com/edgexfoundry/edgex-go/pull/2490
After rebasing, I do not see any issues during functional testing, though I do see an error getting logged (to console logging) for the following PUT command, which has a CBOR payload attached to it: http://localhost:48082/api/v1/device/9f872d68-2281-4af4-959d-29e4d51c2192/command/b349df4a-6c3d-4218-b8bc-7ff8d51ea50d

level=ERROR ts=2020-04-16T20:34:01.895958Z app=device-virtual source=command.go:318 msg="Handler - execWriteCmd: Put parameters parsing failed: \ufffdx\u001aEnableRandomization_BinarydtruedPathx\u001e/api/v1/device/9f872d68/BinarycUrlxBhttp://localhost:48082/api/v1/device/9f872d68/command/7ff8d51ea50dbidx$9f872d68-2281-4af4-959d-29e4d51c2192dnametRandom-Binary-DevicejadminStatehUNLOCKEDnoperatingStategENABLED"

This would point to the fact that, at some point in time, we should also swap ugorji/go with fxamacker/cbor library for device-sdk-go.

Some sample test results:

The work done in this PR stands independently of the work that was done in the device-sdk-go repo, which was for related functionality (to handle both JSON request bodies as well as CBOR request bodies).
That other work can be picked up by anyone interested in running with it. To that intent, I captured relevant details in https://github.com/edgexfoundry/device-sdk-go/issues/488

How are you verifying this? I'm trying to verify using the command black box tests, and while it passes the error case tests, it's hanging on the positive tests.
Here's my steps, which are the same as the standard command black box test run:

redis-cli FLUSHALL
start core-data
start core-metadata
start core-command
start device-virtual
run core-command importer
run collection

Doing this with the error collection works, doing this with the positive collection hangs. Any guidance here on what I'm doing wrong? Are you seeing this locally?

How are you verifying this? I'm trying to verify using the command black box tests, and while it passes the error case tests, it's hanging on the positive tests.
Here's my steps, which are the same as the standard command black box test run:

redis-cli FLUSHALL
start core-data
start core-metadata
start core-command
start device-virtual
run core-command importer
run collection

Doing this with the error collection works, doing this with the positive collection hangs. Any guidance here on what I'm doing wrong? Are you seeing this locally?

Used the following steps for verifying: https://github.com/edgexfoundry/edgex-go/pull/2446#issuecomment-601930622

@brandonforster Do please ping me as and when you have questions regarding the manual validation I've been doing - Also, and in addition to the manual verification steps (for which I've provided a link my reply above), I did also run the core-command black box tests:

@brandonforster @AnthonyMBonafide - With approvals from both you gents, I'll proceed to merge this feature branch before the need arises to go through another rebase cycle 😅


Empty RuleParameter is required if there is a rule has undefined identifier in Expression

Step to reproduce:

Here is the workflow rules:

Noted that the second rule "AvailableIf" contains two more expression identifiers: productStock and customer

[
  {
    "WorkflowName": "DisplayProductAvailability",
    "WorkflowRulesToInject": null,
    "Rules": [
      {
        "RuleName": "AlwaysAvailable",
        "Operator": null,
        "ErrorMessage": "test",
        "ErrorType": "Warning",
        "RuleExpressionType": "LambdaExpression",
        "WorkflowRulesToInject": null,
        "Rules": null,
        "LocalParams": null,
        "Expression": "input.Name == \"Normal product\"",
        "SuccessEvent": null
      },
      {
        "RuleName": "AvailableIf",
        "Operator": "AndAlso",
        "ErrorMessage": "Not Available",
        "ErrorType": "Warning",
        "RuleExpressionType": null,
        "WorkflowRulesToInject": null,
        "Rules": [
          {
            "RuleName": "InStock",
            "Operator": null,
            "ErrorMessage": "Out of stock",
            "ErrorType": "Warning",
            "RuleExpressionType": "LambdaExpression",
            "WorkflowRulesToInject": null,
            "Rules": null,
            "LocalParams": null,
            "Expression": "input.Quantity < productStock.Quantity",
            "SuccessEvent": null
          },
          {
            "RuleName": "VisibleToCustomer",
            "Operator": null,
            "ErrorMessage": "Product is not available at customer location",
            "ErrorType": "Warning",
            "RuleExpressionType": "LambdaExpression",
            "WorkflowRulesToInject": null,
            "Rules": null,
            "LocalParams": null,
            "Expression": "productStock.Location == customer.Location",
            "SuccessEvent": null
          }
        ],
        "LocalParams": null,
        "Expression": null,
        "SuccessEvent": "In stock and visible to customer"
      }
    ]
  }
]

Execute the workflow with one RuleParameter:

dynamic input = new ExpandoObject();
input.Name = "Normal product";
var ruleParameters = new[]
   {
      new RuleParameter( nameof( input ), input )
   };
RulesEngine.ExecuteRule("DisplayProductAvailability", ruleParameters );

Result: An exception is thrown: "Target object is not an ExpandoObject"
My expection is that the RulesEngine run successfully and the RuleResult should be success for rule name "AlwaysAvailable"

Execute the workflow with two RuleParameters:

dynamic input = new ExpandoObject();
input.Name = "Normal product";
var ruleParameters = new[]
   {
      new RuleParameter( "empty", "" ), // empty rule parameter and not related to any rules expressions
      new RuleParameter( nameof( input ), input )
   };
RulesEngine.ExecuteRule("DisplayProductAvailability", ruleParameters );

Result: RulesEngine execute successfully, the first rule name is returned "AlwaysAvailable"

@kamapcrazy Can you try v2.1.1 and see if this issue still persists?

this issue is fixed as a part of 2.1.1, you can refer this test case
https://github.com/microsoft/RulesEngine/blob/6b66162e561b5447c4c3500f167f8823c3c7cbbe/test/RulesEngine.UnitTest/BusinessRuleEngineTest.cs#L220-L234


some german typo for language files

the title says

@mike-ward what is Scrutinizer?

Scrutinizer was something I was experimenting with. Need to remove it.

Thanks, dude! Looks like there were a number of problems in the translation. I thought a native speaker did the original work. Maybe he wasn't.


Add Wikimedia Commons

Hey, is it possible to add the photos of Wikimedia commons to the search results?

I developed the plugin with the intention on focusing on Pexels only, but Wikimedia commons seems like a good source of images!
I will study the API and maybe I can publish another plugin specific to it. Thanks for the suggestion!

You're welcome and thanks for considering it.
Personally, I am not a fan of a plethora of plug ins. So should you be able to integrate it into the current plug in this would be cool. Otherwise you have to switch between 2 plugins if you can't find the image you're looking for in one of them.
The plugin name does not suggest that it is only for looking up pexels images, so I personally don't know why a separate plug-in should be created.
I think it would be better to have 2 checkboxes or a list under the search bar where you can choose where you want to look up your photos pexels wikimedia commons...

Here is the Wikidata Commons API


Tiered allocators

As I rewrite the kernel memory allocator, I should think about incorporating some notion of tiering. We could start with a simple bump pointer allocator early in the init process, and use that for allocating heap objects that live as long as the kernel does (and will never need to be deallocated), and wait to set up the buddy-block allocator until after we've remapped the kernel into the higher address space (since the buddy allocator seems to break occasionally when our paging setup is incomplete).
There's precedent for this sort of tiered allocation scheme; the Linux kernel starts out with a bump pointer allocator at boot time.
We could use an enum to encode one-way allocator state transitions in the type system, so that once the system allocator has been upgraded to the buddy-block allocator, there's no way for it to "go back".
In addition to tiering the system allocator for Rust collections, we should be able to compose slab allocators on top of the main allocator, for constructing specific object types. We should use these slab allocators for kernel objects that we need to allocate a large number of.

Alternative: placement expressions
Another option is to look into using the placement expressions feature (rust-lang/rust#27779) to control where objects are allocated. Rather than making the bump pointer allocator a system allocator that links with the allocator lang items, we could explicitly use it to allocate objects using placement expressions.
Pros & Cons

Makes where objects are allocated explicit at the call site (:thumbsup:)

rather than just using heap-allocated objects and assuming that the allocator crate will put them in the right place, using placement expressions makes allocations explicit in the code that does the allocating. this would help make our code easier to read and conveys more intent.

Getting to use a cool new language feature (:thumbsup:)
Having to deal with an unstable new API (:thumbsdown:)

This is the flip side of the previous point

Wouldn't have to mutate a global static as much (:thumbsup:)

Even if we do still use tiered allocators for the system allocator, we might still want to look into using placement expressions for the kernel slab allocators...


es: support different elasticsearch connections per index

the following change adds the infra changes needed for pointing topology and flow each to a different ElasticSearch database (the extra change need to support this is extending the skydive.yml analyzer.storage to support flow connection and point to this path during NewConnConfig call (for flow).
This would enable extra isolation between the two in productive environments.
For now default behavior (in which both flow and topology take connection configuration from storage.elasticsearch remains).

ok to test

rerun unit-tests

I think i would take #968 furthur, such that i can refer to memory, orientdb, elasticsearch (default entries defined under storage), but can also add more options such as elasticsearch2 (an alternative ES instace) and then i could move limits from analyzer.flow (or analyzer.storage) to storage.elasticsearch, storage.elasticsearch2, etc.

@nplanel / @safchain
The proposed fix has some general code improvements:

moving all initialization to the c'tor (and out of Start() which is an interface method).
adding of helper methods for reading from configuration (and eliminating duplicated reading of configuration parameters)
I could be merged regardless of the separate discussion on changes on skydive.yml storage configuration changes.

If nonetheless you prefer to also address changes in the configuration in the same go - then I would suggest the following:
analyzer:
  storage:
    flow:
      backend: elasticsearch.flow
      max_chan_size: 100000                        # previous max_flow_buffer_size
    topology:
      backend: elasticsearch.default

storage:
  elasticsearch:
    default:
      host: <IP_ADDRESS>:9200
      bulk_max_conns: 10                                 # previously maxconns
      bulk_retry_seconds: 60                             # previously retry
      bulk_insert_size: 100                                 # previously bulk_maxdocs, bulk_insert
      bulk_insert_seconds: 5                              # previously bulk_maxdelay, bulk_insert_deadline
      index_entries_limit: 0
      index_age_limit: 0
      indices_to_keep: 0
    flow:
      ...
  orientdb:
    ...

@eranra @hunchback @safchain
1/ Would be better to not mix backend and indexes, there are two different things
2/ Let's keep the configuration simple and backward compatible

storage.elasticsearch define the default configuration (as today)
storage.elasticsearch.{index} define the {index} configuration (if exist, else fallback to default)
3/ analyzer flow_server bulk_insert and bulk_insert_deadline, use the configuration values according to the selected flow backend

analyzer:
  storage:
    flow:
      backend: elasticsearch
      max_flow_buffer_size: 100000
    topology:
      backend: elasticsearch

storage:
  elasticsearch:
    host: <IP_ADDRESS>:9200
    maxconns: 10
    retry: 60
    bulk_maxdocs: 100
    bulk_maxdelay: 5
    index_entries_limit: 0
    index_age_limit: 0
    indices_to_keep: 0
    flow:                                                           # optional
      host: <IP_ADDRESS>:9200
      ...
    topology:                                                    # optional
      host: <IP_ADDRESS>:9200
      ...
    snort:                                                          # optional
      host: <IP_ADDRESS>:9200
      ...
  orientdb:
    ...

@nplanel  (@safchain, @eranra) - proposed config seem fine, can this be handled in a separate PR?

I think the config need to be handled in the this PR as it's related to ES and indexes.

Ok
On Sat, Apr 21, 2018, 12:53 Nicolas PLANEL<EMAIL_ADDRESS>wrote:

I think the config need to be handled in the this PR as it's related to ES
and indexes.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/skydive-project/skydive/pull/962#issuecomment-383282160,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACogI-WBvDQkhHl90PJrmBaN6ic0B1idks5tqwG2gaJpZM4TYkOE
.

@nplanel - I have just uploaded an updated proposal - to handle default values I and added a notation with '?' such as storage.elasticsearch?flow.host which means check if storage.elasticsearch.flow.,host is set if not then fallback to storage.elasticsearch.host

@hunchback
why adding "?" ?
why not try to resolve the key like
host := ""
if host = cfg.GetString("storage.elasticsearch.flow.host"); host == "" {
    host = cfg.GetString("storage.elasticsearch.host")
}

I am using the '?' notation so as to keep the calling code simple.
Additionaly, by adding the '?' notation i was able to extend all existing
config.GetXXX(key string) functions with only minimal code changes (and
minimal added complexity)
On Mon, Apr 23, 2018, 19:47 Nicolas PLANEL<EMAIL_ADDRESS>wrote:

@hunchback https://github.com/hunchback
why adding "?" ?
why not try to resolve the key like
host := ""
if host = cfg.GetString("storage.elasticsearch.flow.host"); host == "" {
host = cfg.GetString("storage.elasticsearch.host")
}
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/skydive-project/skydive/pull/962#issuecomment-383643815,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACogI-jEHMsWMl8VLwwlsU4SCZs2yyELks5trgWDgaJpZM4TYkOE
.

I will fix the handling of deprecated keys according to example #968
On Mon, Apr 23, 2018, 20:10 Nicolas PLANEL<EMAIL_ADDRESS>wrote:

@nplanel commented on this pull request.
In config/config.go
https://github.com/skydive-project/skydive/pull/962#discussion_r183470284
:

@@ -89,12 +104,10 @@ func init() {
cfg.SetDefault("analyzer.packet_injection_enabled", true)
cfg.SetDefault("analyzer.replication.debug", false)
cfg.SetDefault("analyzer.ssh_enabled", false)

cfg.SetDefault("analyzer.storage.bulk_insert", 100)
cfg.SetDefault("analyzer.storage.bulk_insert_deadline", 5)
cfg.SetDefault("analyzer.storage.index_age_limit", 0)
cfg.SetDefault("analyzer.storage.index_entries_limit", 0)
cfg.SetDefault("analyzer.storage.indices_to_keep", 0)
cfg.SetDefault("analyzer.storage.max_flow_buffer_size", 100000)

cfg.SetDefault("analyzer.storage.flow.bulk_insert", 100)

please take a look at #968
https://github.com/skydive-project/skydive/pull/968
graph.backend => analyzer.topology.backend
analyzer.storage => analyzer.flow
analyzer.flow.{bulk_insert,bulk_insert_deadline,max_flow_buffer_size} must
be removed from the configuration and deduced/resolved from the used
backend (analyzer.flow.backend)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/skydive-project/skydive/pull/962#pullrequestreview-114474467,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACogI6XPUY1rB4fKFSPFFMWpHY5eLU6Lks5trgrxgaJpZM4TYkOE
.

I am using the '?' notation so as to keep the calling code simple.
Additionally, by adding the '?' notation i was able to extend all existing
config.GetXXX(key string) functions with only minimal code changes (and
minimal added complexity).
Compare:
retry := 0
if val := cfg.Get("storage.elasticsearch.flow.retry"); val != nil {
    retry = val.(int)
} else {
    retry = cfg.GetInt("storage.elasticsearch.retry")
}

To
retry := cfg.GetInt("storage.elasticsearch?flow.retry")

In our case things are ever more acute as we have a helper function handling the calling of configuration which is also simplified to:
In your case it would be:
cfg := es.NewConfig("storage.elasticsearch.flow", "storage.elasticsearch")

Simplified to:
cfg := es.NewConfig("storage.elasticsearch?flow")

@hunchback the only call you need to do is
limits := elasticsearch.NewElasticLimitsFromConfig("storage.elasticsearch", "flow")

or even you can simplify

limits := elasticsearch.NewElasticLimitsFromConfig("flow")
limits := elasticsearch.NewElasticLimitsFromConfig("topology")
...
limits := elasticsearch.NewElasticLimitsFromConfig()

@nplanel - I agree there is little difference between:
limits := elasticsearch.NewElasticLimitsFromConfig("storage.elasticsearch", "flow")

And:
limits := elasticsearch.NewElasticLimitsFromConfig("storage.elasticsearch?flow")

But this is a negligible aspect - the main issue is config.GetInt("storage.elasticsearch?flow.retry") and friends if not handled seamlessly by config then we have a 7-8 way code duplication, and this will only get worse we we use the default/fallback trick elsewhere in the code (in storage.orientdb for example).

Tests failed due to:
2018-04-24T07:59:03.582Z	DEBUG	helper/helper.go:135 ExecCmds	skydive-slave.rdocloud: Executing command [lxc launch images:fedora/27 test-skydive-lxd-simple]
2018-04-24T07:59:03.628Z	DEBUG	helper/helper.go:138 ExecCmds	skydive-slave.rdocloud: Command returned Connection refused; is LXD running?

@nplanel  - can you check was is the matter?

run skydive-functional-tests-backend-elasticsearch

I'm not a fan of the "?" notation and also prefer ```
cfg := es.NewConfig("storage.elasticsearch.flow", "storage.elasticsearch")

One way would be to make the GetString, GetInt take a ```...string``` argument and returns the first that is set, but I'm not sure it's worth it.

There are changes in your PR related to the proposal, but it seems the PR is not rebased of it, that makes it a bit hard to understand.

I thought about extending config.GetInt() and friends ... but this has two
shortcomings:

Usage is more comborsum, eg: GetInt("storage.elasticsearch.flow.retry",
"storage.elasticsearch.retry") compared to
GetInt("storage.elasticsearch?flow.retry")

Extending interfaces will cascade down to extending each functions'
implementation ...

So i don't see any advantage of this over the '?' notation .. and would
much prefer we stay with my current implementation. Also note this notation
is not exposed to users - only used from within the code.
As to your comment on rebase - please elaborate
On Apr 24, 2018 6:34 PM, "Sylvain Baubeau"<EMAIL_ADDRESS>wrote:

I'm not a fan of the "?" notation and also prefer ```
cfg := es.NewConfig("storage.elasticsearch.flow", "storage.elasticsearch")
One way would be to make the GetString, GetInt take a ...string argument and returns the first that is set, but I'm not sure it's worth it.
There are changes in your PR related to the proposal, but it seems the PR is not rebased of it, that makes it a bit hard to understand.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/skydive-project/skydive/pull/962#issuecomment-383977471,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACogI_TqEH-22uFlgJoEFFhfnyhuzDQeks5tr0YRgaJpZM4TYkOE
.

It's not clear what GetInt("storage.elasticsearch?flow.retry") (at least for me). And it only allows simple use cases such as "look inside the child then in the parent if you didn't find it".
GetInt("storage.elasticsearch.flow.retry", "storage.elasticsearch.retry") seems a bit clearer on what it does and allows the keys to be completely different.
That being said, maybe we should the KISS approach and go with:
config := cfg.Get("storage.elasticsearch.flow")
if config != nil {
    ...
} else {
    config = cfg.Get("storage.elasticsearch")
}

as it's only used once for now. We'll still be able to generalize that if we use it for something else. What do you think ?

@lebauce - the snipped you provided will fail generally, as we want the ability per each parameter to get the value first from "storage.elasticseach.flow" and fallback to "storage.elasticsearch". For example:
storage:
  elasticsearch:
    retry: 10
    flow:
    # no retry value

In your snippet you will incorrectly return 0 rather than 10
In effect extending the key syntax provides a means of extending capabilities while only having to "fix" one function - realKey() - so this is the generic way forward.

@lebauce - I am not very happy with the last commit (below), I would much rather drop it ...
https://github.com/skydive-project/skydive/pull/962/commits/dc3f024832f2796a8651daf0f72e01df464c8215

Failed due to:
--- FAIL: TestPacketInjectionCapture (136.31s)
	selenium_test.go:177: By default 8 rows should be return, but got: 3
FAIL

Which does not appear related to my fix

run skydive-selenium-tests

can this be merged ?

Selenium is failing:
5b2ce5b57e9b: Pull complete
Digest: sha256:df5655cafb23744708d85653156b295979da25eb11eb85b4498219ed38212a99
Status: Downloaded newer image for docker.io/elgalu/selenium:latest
...
...
2018-05-02T16:38:23.576Z	INFO	ovs/ovsdb.go:194 (*OvsMonitor).interfaceUpdated	jenkins-slave-5.rdocloud: Interface "vm2-eth0(ac79407d-c37b-4e4f-b0c6-05519a89c807)" updated
panic: test timed out after 10m0s

@nplanel - if key is not deprecated and a "" is returned by isDepKeySet()
then firstKetSet() checks if key is set - if so then key is returned by
firstKeySet()
On Wed, May 2, 2018, 21:57 Nicolas PLANEL<EMAIL_ADDRESS>wrote:

@nplanel commented on this pull request.
In config/config.go
https://github.com/skydive-project/skydive/pull/962#discussion_r185603606
:

// check is there is a deprecated key that can be used

depKeys, found := relocationMap[key]
if !found {

return key

return ""

you break all good keys not relocated ....
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/skydive-project/skydive/pull/962#pullrequestreview-117025954,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACogI51SChSsdgygsCW-N7VvDuc-vd5cks5tugGzgaJpZM4TYkOE
.

run skydive-functional-tests-backend-orientdb

run skydive-selenium-tests

@nplanel / @safchain / @lebauce  - please review https://github.com/skydive-project/skydive/pull/962/commits/f9e3e066d1cd2d44a997f0ba35825b52d3091797


Versions, citations and references for tools being used

Is your feature request related to a problem? Please describe.
I am currently writing a manuscript that uses snakemake with a lot of wrapper scripts, and have to go through all tools individually to get (a) the exact version of the tool, and (b) then look up its reference for the bibliography.
My process is:

Go through all my snakemake files and all rules and tools being used. For each of them:
Find the wrapper version:wrapper:
        "0.51.3/bio/trimmomatic/pe"

Find the documentation at that exact version of the wrapper: https://snakemake-wrappers.readthedocs.io/en/0.51.3/wrappers/trimmomatic/pe.html
Get the version of the tool being used from there.
Do a Google Scholar search for the tool to get its preferred citation/reference.
Then, put this into text: "We used trimmomatic v0.36 [1]", etc...

This is repetitive work, not only for me, but for everyone who is in a similar situation. The two pieces of information that I am interested in here are the version of the tool being used, and its reference.
Describe the solution you'd like
Ideally, snakemake could offer some command that takes a config file/workflow, and figures out all tools being used in that exact workflow configuration (skipping any rules that are not being invoked!). It then automatically collects the needed information, and prints it in some format.
Tool version information is already available via the environment.yaml in the wrapper, and citation or links to the tool websites could be added (optionally and bit by bit for each tool) to the meta.yaml and read from there.
I guess that this is only feasible for wrapped tools, and that I still have to go through my individual (shell/script based) rules. But the above snakemake command then could at least also list all those rules, so that I do not forget about them. Basically, it prints out a linear list of the dependency graph (similar to the normal terminal output when running snakemake, but only once for each rule), but with all tools and references listed.
So, my ideal solution would look something like this:
$ snakemake [...] --print-tools # or whatever you would want to call this command
Rule `trim_read`:
    - Uses wrapper `TRIMMOMATIC PE`
    - Tool: trimmomatic v0.36
    - Conda: https://anaconda.org/bioconda/trimmomatic
    - URL: http://www.usadellab.org/cms/index.php?page=trimmomatic
    - Reference: Bolger, A. M., Lohse, M., & Usadel, B. (2014). Trimmomatic: A flexible trimmer for Illumina sequence data. Bioinformatics, 30(15), 2114–2120. https://doi.org/10.1093/bioinformatics/btu170
Rule `my_rule`:
    - Uses shell-based command
Rule `my_other_rule`:
    - Uses python-based command
....

That would be awesome!
Describe alternatives you've considered
I thought that a simple overview table here in the repo might also be a solution, but that would get too messy rather quickly, with all wrappers in different versions etc... and it would not really speed things up, would be extra maintenance work, and would probably be outdated all the time, and... No, not a good alternative.

I have also opened this issue in the snakemake wrapper repository. Feel free to close one of them - I wasn't quite sure what the best place to put this is.

Hi @lczech ,
I am just seeing this now, sorry for the slow response. An overview table of all the tools used and the respective versions sounds like the perfect thing to add to snakemake --report report.zip when creating a snakemake report? Even if you do not mark any specific rule output for inclusion in the report, the basic report will give you an overview of all the rules executed in DAG-form and run time statistics. But it should also be possible to systematically parse the (latest) conda environments and write a table with all the versions. Optimally, that table should be exportable into different formats (tsv, some open spreadsheet format). Sadly, I don't currently have time to pick this up, but I'll mention it in my group, and maybe someone else who sees this (including you;) can find some time to pick this issue up!
Also, as I think this best fits here, I'll close it over at the snakemake wrappers. But feel free to reopen, if something on the snakemake wrappers repo also needs to change.