[Service Bus] Update Atom API operation settable parameters

Based on findings, it turns out that the service isn't supporting update of several parameters that are considered as optional in implementations of the Atom API feature.
The expected ordering of parameters in the XML body is not documented and needs investigation. This is to be done for all 20+ operations with 10+ possible input parameters on each.
One round of this investigation has been factored into current implementation of feature in Typescript SDK as part of #4922
This issue is to investigate each of the parameters more closely and surface them when cofirmed that the service does support them.
The results of this work should help improve the user experience on other language SDKs as well.

Updates:

Looked into .NET and Python libraries and the parameters are getting set by the service. Many of these are missing from the older SDK and can be updated.
However, "enablePartitioning" is not getting set, even when the request.body in XML is exactly the same as one that works when sending via .NET SDK. Looks like there might be something else going on with the headers, as the target Service Bus instance is the same as well. Needs more investigation.
Parameters like MessageCount and SizeInBytes are updated by the service and was being updated by the Python SDK. (Although there implementation of the feature seems to be in PR and not on master yet). More investigation may be needed on this with service team on if these should be exposed to user or not. Keeping Javascript users in perspective, we may have to explicitly disallow if these should not be updated. (As, at first glance they seem like dynamic, retrieve only properties based on state of entity).
Other good to have next steps are to investigate each of the properties and document few examples of default values, or atleast have references to service side documentation on the parameters if available.

cc @ramya-rao-a @AlexGhiondea @chradek

Updates:

Enable partitioning issue is resolved! It was to do with just using the targeted Standard pricing tier.
Rule deserialization works, and verified that the XML generated by .NET SDK works but not in JS SDK. Next steps are to update code and serializers, so that the expected request XML get generated.

Pending:

Update create rule serialization to match with .NET SDK
Similar efforts are needed for AuthorizationRule serialization
To look deeper into properties EnableExpress, IsExpress, IsAnonymousAccessible
Verify with service tea if MessageCount, SubscriptionCount and SizeInBytes are indeed not settable.
To look into unknown properties introduced in JS SDK but which don't seem to map to any on service side or other SDKs - confirm with service on this.

@ramya0820 Are the pending items in the above comment still applicable or are there any updates?

Another point to investigate and likely discuss with service team on how best to handle are about properties that don't get updated as expected. These are documented, noted in the management API tests.

@ramya0820 Are the pending items in https://github.com/Azure/azure-sdk-for-js/issues/5354#issuecomment-538583879 still applicable or are there any updates?

I've updated the comment with notes inline.
Overall the pending items that I plan on looking into and work with service team on confirming about are:

Clarify on if there is any recommended order of properties that we are to use, which maybe different from .NET SDK (Can double check take notes on if .NET behaves differently for some of these properties)
Discuss with service team on how best to handle properties that don't get updated as expected. These are documented, noted in the management API tests.
-   if MessageCount, SubscriptionCount and SizeInBytes are indeed not settable.
-   Enlist and clarify on JS SDK specific custom properties that were introduced but don't get set to anything on service side.
-   autoDeleteOnIdle and forwardDeadletterMessagesTo properties affecting each other
-   Deeper analysis on properties EnableExpress, IsExpress, IsAnonymousAccessible

We decided to try and maintain parity with .NET SDK and investigate user experience collectively later

#6395 updates settable parameters on Queues, Topics, Subscription and related validation. Specifically, adds support for  status input.

Other pending things are:

Investigating support for normalizing URLs so that expected user input format is same across SDKs for URL based properties
Investigate working of “DefaultRuleDescription” input as it appears to be broken
Investigate use of ‘unknown properties’ for making things “forward proof”
Potentially remove unused parameters from -Details/ output

#6395 has been updated to address above pending items. Please see description of the PR for more notes
Another point is that we could try and maintain shape parity with ARM Service Bus package on the properties naming. https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/servicebus/arm-servicebus/src/models/mappers.ts
Thoughts? This might make us want to revert some of the naming improvements we have been thinking of so far
cc: @ramya-rao-a @bterlson @AlexGhiondea

Closed with https://github.com/Azure/azure-sdk-for-js/pull/6395
Also discussed offline regarding the naming improvements concern - consensus is this isn't a major story and we are comfortable with making the improvements in the data plane SDKs

@ramya0820 There are multiple places in the atomManagement.spec.ts file where there is references to To be investigated further as part of https://github.com/azure/azure-sdk-for-js/issues/5354
I assumed that this issue was going to be used for further investigations. Therefore, re-opening this issue.
If the comments were stale and don't apply anymore, please send a PR to clean them up and we can close this issue again.

Addressed this as part of https://github.com/Azure/azure-sdk-for-js/pull/6382/commits/bb5fa475133e1e5f4452223b218c670a8fa6062c since this PR is next in queue and just absorbed changes from #6395. @ramya-rao-a

Closed with https://github.com/Azure/azure-sdk-for-js/pull/6382


Avoid loading the GeoJSON data multiple times

"...requestData calls in other place." A small change in the viewport may trigger a tile download for some sources, but it won't trigger anything for geojson sources. The filtering is source dependant.


added blendModes to depracation

Mind adding scaleModes as well since you're in there?

Love it, you're the best!


Published crate fails to compile tests due to absence of jemalloc-ctl

As part of my efforts to improve linux vendorization of rust crates, which includes the ability to test a crate, I've stumbled onto this problem:
jemallocator-0.3.2 $ cargo test
    Updating crates.io index
  Downloaded jemalloc-sys v0.3.2
  Downloaded cc v1.0.47
  Downloaded fs_extra v1.1.0
   Compiling proc-macro2 v1.0.6
   Compiling unicode-xid v0.2.0
   Compiling syn v1.0.8
   Compiling cc v1.0.47
   Compiling fs_extra v1.1.0
   Compiling libc v0.2.65
   Compiling jemalloc-sys v0.3.2
   Compiling quote v1.0.2
   Compiling proc-macro-hack v0.5.11
   Compiling paste-impl v0.1.6
   Compiling paste v0.1.6
   Compiling jemallocator v0.3.2 (/home/kent/.cpanm/work/1574930033.31685/jemallocator-0.3.2)
error[E0463]: can't find crate for `jemalloc_ctl`
 --> tests/malloctl.rs:1:1
  |
1 | extern crate jemalloc_ctl;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

error[E0463]: can't find crate for `jemalloc_ctl`
 --> tests/background_thread_defaults.rs:3:1
  |
3 | extern crate jemalloc_ctl;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: could not compile `jemallocator`.
warning: build failed, waiting for other jobs to finish...
error: could not compile `jemallocator`.
warning: build failed, waiting for other jobs to finish...
error: build failed

Ideally, one of these solutions should be employed:

tests that require jemalloc-ctl are also pruned from the published crate to avoid spurious failures
jemalloc-ctl is declared a dev-dependency on the published crate
Some kind of glue is published in the crate to avoid the needed dependency.

The dependency change for a jemallocator test is the following:
jemallocator->jemalloc-ctl->jemallocator-tests

However, cargo has a bug in which it considers dev-dependencies to be direct dependencies, so you end up with a cyclic dependency of the form:
jemalloc-ctl->jemallocator->jemalloc-ctl

and such dependencies cannot be published to crates.io, so we need to hack the Cargo.toml manually before every release before publishing to crates.io (which has other problems, like the source code that you can see here does not match the source code on crates.io).
Since there are many crates having this problem, and having to manually edit Cargo.toml before release to work around the many flaws and bugs in Cargo's dependency resolution, I don't think it makes much sense to try to solve this problem here.
The fix is simple: dev-dependencies are not dependencies of a crate, but dependencies of its "dev crates" (e.g. tests, examples, benches, etc.), so it should be possible for a dev-dependency to depend on the crate that uses it.
This cargo bug has been open for a long time: https://github.com/rust-lang/cargo/issues/4242

Closing this as "not actionable", since there is nothing we can do to fix this cargo bug here.

Curious, I battle with circular dependencies every day with crates.io stuff in dev-dependencies, clearly these crates aren't blocked by this.
In general I just despise circular dependencies, so ain't going on a bender to encourage more of these.
But I also suspect there's a good compromise that doesn't require circular dependencies.

As mentioned, there is no circular dependency in jemallocator, but cargo has a bug that makes it think that there are. If you believe this is incorrect, and that there is a circular dependency in this crate, please, go ahead, and let us know about it.
But IIUC, what you want is for us to fix a circular dependency issue that actually does not exist. Your time is better spent fixing the cargo bug that makes cargo think that there is a circular dependency here. Complicating this crate, or other crates, to work around that cargo bug, makes the situation worse.

There is a circular dependency though. Just because it's a dev dependency doesn't make the circularity go away.
OTOH, not doing anything means vendors such as distros cannot blindly run tests, which sounds pretty bad for a thing as important as a memory allocator.
And neither of @kentfredric suggested fixes would complicate the crate in that book, in my opinion. Au contraire, it will remove a circular dev dependency, which is definitely a simplification.

Fwiw, I locally patched in the dependency in jemallocator, and everything works JustFine for me, can you show what problem you're having in a different way? So far your descriptions of the problem are just confusing me :/

jemallocator depends on jemalloc-ctl
jemalloc-ctl depends on jemallocator

Right? Am I missing something obvious here?

Right? Am I missing something obvious here?

jemallocator does not depend on jemalloc-ctl. jemallocator-tests-crates in the test/ directory do, and those are different crates than the jemallocator crate.

Wrong: jemallocator does not depend on jemalloc-ctl. jemallocator-tests-crates in the test/ directory do, and those are different crates than the jemallocator crate.

I literally linked you to the jemalloc-ctl dev-dependency of jemallocator.

I literally linked you to the jemalloc-ctl dev-dependency of jemallocator.

I know, you linked there, but incorrectly claimed that this means that jemallocator has a dependency on jemalloc-ctl, while cargo devs have acknowledge that this is not the case, and the current cargo logic is due to a bug.

My claim is definitely correct, we disagree on the consequences of that fact but I don't appreciate being told things that are false.
Where is the Cargo issue?

Where is the Cargo issue?

I linked it in my reply to this post, where I explained this in detail.

So my original point stands: there is a cyclic dev-dependency, and cyclic dependencies, be them dev or not, are a huge increase in complecity and process. Removing the cycle would simplify things, especially for vendors and distros.

@nox No, your original point is still incorrect.
The jemallocator crate does not depend on jemalloc-ctl.
A Cargo.toml describes multiple crates. A test crate within the jemallocator repository depends on both jemallocator and jemallocator-ctl, and cargo incorrectly interprets this as jemallocator having a cyclic dependency, and this is an acknowledge bug.
If you disagree that this is a bug, its pointless to try to argue with me here, go and argue with the cargo maintainers upstream that disagree with you.

jemallocators tests require jemalloc-ctl though, this is important.
Even though you're talking about crates as the sense of "compilation unit", there's also the sense of crates as "bundled aggregate of source code".
And there is no way to stipulate dependencies for source code among those without using the top level Cargo.toml
Thus, stipulating dependencies for tests must be done in Cargo.toml, the only problem here I'm seeing is that "dev-dependencies" is over-general, when what you need is "test-dependencies".
So while terminologically correct that "test/" and "jemallocator" are "different logical crates", jemalloc-ctl is still a "test dependency" of the crate in question, as it is required to "compile and run the tests of the crate". ( The tests themselves just happen to be different compile-unit crates, but that's an implementation detail )

Even though you're talking about crates as the sense of "compilation unit", there's also the sense of crates as "bundled aggregate of source code".

That's a package, not a crate.

Fwiw, I locally patched in the dependency in jemallocator, and everything works JustFine for me, can you show what problem you're having in a different way?

What patch did you apply? Notice that our Cargo.toml has a dev-dependency, which corresponds to your second proposed solution, and that works just fine, with the exception that cargo doesn't allow such crates to be uploaded to crates.io by default.

To side step a lot of faff:
Can you show us ( eg: the error you get, cargo whining etc ) what happens when jemallocator's Cargo.toml includes the section:
[dev-dependencies.jemalloc-ctl]
version = "^0.3"
default-features = false

Because I don't see any issues when I do this, and this is all that's required to get things working.

The error only happens when you try cargo publish.

I would like to see this, because as it is, there are so many crates currently and recently published on crates.io with messy circular dependencies, that it conspires to be a bit of a nightmare.

I would like to see this,

The execute cargo publish ?

At some point I may take a stab at writing a patch that makes this less painful in some regards.
Though I'll probably take the path of putting the tests that require circular dependencies in such a location that you can exclude them automatically with an exclude =  rule.
This at least would make things usable.

Would those tests then run ?

Those tests would continue to run on the git clone, but would be non-existent ( and thus, couldn't fail to compile ) in the published crate.

But this is already the case ?

No, not at all, that's why I had to file this bug.

So I'd like to see your patch, because the master branch already has a dev-dependency that makes the tests work.

Ping me when you publish next then, and if it still poses a problem I can look into it :)
If the change to Cargo.toml currently in master fixes the problem, then I don't need to :)

As mentioned a 100 times, cargo publish fails for master, because of the dev-dependency key, so it must be removed before publishing, so that crates.io accepts it. I'm unsubscribing from this thread, since I don't have time for this, and I feel my time isn't being valued.

And again, as long as the published crate works, I won't have a problem.
That's why I'm going to look into a solution that works for published crates.
"The fix is in master" doesn't translate to "the fix is on crates.io"
:)


dynamic Details, Active, Inactive, Selected possible in SelectTemplates?

Hey,
thx for the awesome module!
i like to ask, if it is possible to create a dynamic/conditional Details String.
my code looks like that (mostly copied from the exampletemplates, i am a go beginner):
pathesChoose := &promptui.SelectTemplates{
	Label:    "{{ . }}?",
	Active:   "\U0001F336 {{ .Type | cyan }}: ({{ .Value | red }})",
	Inactive: "  {{ .Type | cyan }} ({{ .Value | red }})",
	Selected: "\U0001F336 {{ .Type | red | cyan }}",
	Details: `
--------- Path ----------
{{ "Type:" | faint }}	{{ .Type }}
{{ "Value:" | faint }}	{{ .Value }}
`,
	}
	prompt := promptui.Select{
		Label:     "Select Path",
		Items:     pathes,
		Searcher:  pathSearcher,
		Templates: pathesChoose,
	}

the array with maps i am passing to Items could look like that:
[
  {name: "1", type: "1"},
  {name: "2", type: "2", additionalValue1: "3"},
  {name: "3", type: "2", additionalValue2: "val"},
]

What i am searching for is a solution to make the Deails Template dynamic, dependent on the keys, the corresponding item has.
For example like that:
Details: `
--------- Path ----------
{{ "Type:" | faint }}	{{ .Type }}
{{ "Value:" | faint }}	{{ .Value }}
{{ if .AdditionalValue1 | {{  "AddVal1:" }} {{ .Value }}
{{ if .AdditionalValue2 | {{  "AddVal2:" }} {{ .Value }}
`

The same would be nice for the Active, Inactive and Selected field.
So my question is, is this already somehow possible?
If not, does anyone has an Idea how to create a workaroung?
thx in advance!

@divramod may be this link could help you (at least it did for me).
In my case I wrote something for a map of K/V (Tags) like:
--- excerpt of code

public name: {{ .PublicName }}
{{ if index .Tags "Project" }}project: {{ index .Tags "Project" }}{{ end -}}
{{ if index .Tags "UserName" }}user: {{ index .Tags "UserName" }}{{ end -}}
{{ if index .Tags "UserEmail" }}email: {{ index .Tags "UserEmail" }}{{ end -}}`,

even better so that everything is on different lines for real:
public name: {{ .PublicName }}
{{ if index .Tags "Project" }}project: {{ index .Tags "Project" }}
{{ end -}}
{{ if index .Tags "UserName" }}user: {{ index .Tags "UserName" }}
{{ end -}}
{{ if index .Tags "UserEmail" }}email: {{ index .Tags "UserEmail" }}
{{ end -}}`,

thx!


Support for 5.2?

Would you accept a PR to move to the newly released 5.2 version? The images appear to be on dockerhub already.

@laidig Images are using the tag 5 which point to 5.2. Just do a docker-compose pull to update your stack.
Re-open this issue if you're still having problems.

I'll re-open the issue as I just discovered that 5.2 introduced logstash-plugin https://www.elastic.co/guide/en/x-pack/5.2/monitoring-logstash.html. This is not available in the current x-pack setup. I'd accept a PR for that.

Related to #95. Maybe enabling logstash-plugin should be the topic of another issue?

5.2 support introduced via #95


Updates configuration templates to use json structure

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.11%. Comparing base (a104b55) to head (e316a49).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #58   +/-   ##
=======================================
  Coverage   94.11%   94.11%
=======================================
  Files          27       27
  Lines        1087     1087
=======================================
  Hits         1023     1023
  Misses         44       44
  Partials       20       20

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


Adding default config.toml file for Kubos services

From the Trello card:
Right now each KubOS Service (hardware or not) reads it's own address and port number from a section in `/home/system/etc/config.toml`. Some services (like telemetry) will actually fail to start if the config section is missing, others will fall back to `<IP_ADDRESS>:8080`.

Long term, we need to explicitly state which service lives on what port so that there are no conflicts or user configuration required out of the box.

So I copied the current version of our config.toml into the common/overlay/home/system/etc folder with the expectation that any time a new service is made and the KLB package for it is created, this file should be updated to add the new default configuration options.
Users can then modify this file to remove or change any options they like before building it in to their KubOS image.

 Create a default system-level config.toml

Well that's tricky. Honestly I think at least having it manually made is fine for now. We should have a config.toml that includes all services in it, and people can just remove what they aren't using. It's an extra step, but a rather trivial one. Especially because you won't really be changing it after that.


Parse event URLs without a trailing slash

Fixes #165

Looks good.
I know that a foolish consistency is the hobgoblin of little minds, but wonder whether it'd also be worth fixing the link in schedule.rst.

You're welcome to, but I won't be doing so


Fix test failures on Windows for time zone and remoting

PR Summary

Fix test on Windows client where -SkipNetworkProfile check is needed.
Make Test-IsWinServer2012R2 a common help function and skip time zone tests correctly.

PR Context

PR Checklist

[ ] PR has a meaningful title

Use the present tense and imperative mood when describing your changes

[ ] Summarized changes
[ ] Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
[ ] This PR is ready to merge and is not Work in Progress.

If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.

Breaking changes

[ ] None
OR
[ ] Experimental feature(s) needed

[ ] Experimental feature name(s):

User-facing changes

[ ] Not Applicable
OR
[ ] Documentation needed

[ ] Issue filed:

Testing - New and feature

[ ] N/A or can only be tested interactively
OR
[ ] Make sure you've added a new test if existing tests do not effectively test the code changed

Tooling

[ ] I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
OR
[ ] I have considered the user experience from a tooling perspective and opened an issue in the relevant tool repository. This may include:

[ ] Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode
(which runs in a different PS Host).

[ ] Issue filed:

[ ] Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.

[ ] Issue filed:

[ ] Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).

[ ] Issue filed:

[ ] Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

[ ] Issue filed:

@anmenaga - can you please review?


Cairo.Path is leaking, programmer is missing a call to Dispose

The message is shown in application output when running on Linux/Gtk in MonoDevelop.

We need to work what to do here. Problem is that controls are never Disposed of, but Shape has a reference to a cairo path. Maybe it should be disposed when removed from visual tree? Would this fix the message?

I do not use the Shape for rendering geometry and text.
I'm overriding the Render method for Control class:
https://github.com/Core2D/Core2D/blob/master/Core2D.UI.Perspex.Windows/Controls/Drawable.cs#L370
and doing rendering using DrawingContext:
https://github.com/Core2D/Core2D/blob/master/Core2D.UI.Perspex.Windows/Renderer/PerspexRenderer.cs
I can use caching for Pen's and Brushes' but from time to time (when style has changed in my program on new project was loaded) I need to dispose them.

I think this also is important for other platforms, so the pattern of acquiring and disposing drawing resources is consistent.


Minor Docs fixes in reviewing decorator

Issue or goal of PR:
Fix minor docs issues (typos/links)
What I did
fixed 2 typos and a broken link
How to test
look at diffs, build docs and verify changes. Verify I got the right intended destination for Extensions

[ ] unit tests were written / updated

Documentation
Check one:

[ X] I updated documentation
OR
[ ] No documentation updates required

Can you resolve the conflicts? Or do you need me to handle?

I'll rebase my few changes onto main and make a different pull request. I thought that the whole decorators branch had been merged into main. Instead, it's trying to bring all of that in here.


New package: easyplotting v1.0.0

Registering package: easyplotting
Repository: https://github.com/srgk26/easyplotting.jl
Created by: @srgk26
Version: v1.0.0
Commit: bb93623cdd8c60abd2b5084f274301cfc169c15c
Git reference: master

Hi! I just made a change to the package name (I capitalised it) from easyplotting to Easyplotting, subsequently capitalised the module name and other corresponding names in the code. I can't seem to have this change reflected on this pull request. Therefore, please reject this PR, I will submit a new PR. Thank you!


What comes before v1.0?

With these requirements addressed in the current draft of the signing-spec, it's likely time to discuss what needs to be addressed before we're ready to tag v1.0. Achieving stability in the spec would enable us to work on the adoption in-spec in ITE-5 / TAP-TBD, and in-code via work on signing-spec implementations.

Tagging: @MarkLodato @SantiagoTorres @lukpueh @trishankatdatadog @mnm678 @joshuagl @JustinCappos

FYI: I just tagged v0.1.0 -- https://github.com/secure-systems-lab/signing-spec/releases/tag/v0.1.0

Do we have working prototypes somewhere, especially one to support Google's use case, and another to support our Canonical JSON? That would be the only thing that would really convince me, not endless specifications.

I suggest creating a GitHub milestone for this. (If you add me as an editor in the project, I'd be happy to set that up for you.)
I filed #28 for the reference implementation.

I've created the milestone and added #28 to it.

Do we have working prototypes somewhere, especially one to support Google's use case, and another to support our Canonical JSON? That would be the only thing that would really convince me, not endless specifications.

I don't know if this is a "reference" implementation, but we have "an" implementation here: https://github.com/in-toto/in-toto-golang/blob/master/pkg/ssl/sign.go

I propose that the following are the only blockers:

#16 Choose a more specific name than "signing-spec"
#27 Use a simpler PAE
#28 Create a reference implementation and test vectors

Once we have done these three, we tag it as 1.0. Any objections?

How much do were care about #27?  There's already an implementation that uses the already defined PAE.
I'm fine either way, I just want to make sure this is actually a good use of time.

IMO it's valuable since it significantly simplifies the implementation and is an easy change to make.

SGTM

I chatted offline with @MarkLodato on the possibility to use a simpler encoding for the envelope part too: that would prevent recipients from using a full-blown json parser on untrusted input. But it's yet more work to make this change and may not best use of time at this stage.

It's 1.0 now!


DecimalInput - Issues with decimal input

There's an issue when entering a decimal point using the numeric keyboard in Edge or IE. Using the .  in the text area of the keyboard works as expected, but using the . on the numeric area doesn't work. This is fine in all other tested browsers, and is fine when triggering it using the triggerKeypress function in the API.

Should be resolved by #6

As you correctly identified, it's a bug in IE that reports the Event.key of the '.' key as delete. This will unfortunately need an IE specific fix.

Fixed for now by #6 however there is a potential future issue if Microsoft remove Event.charCode support before fixing their implementation of Event.key. Settled with this fix because the only alternative would require user agent sniffing which is gross.


[release-v1.18] Extend list of non-retryable error codes

Cherry-pick of #3662.
Release note:

When a shoot is erroring with `ERR_INFRA_INSUFFICIENT_PRIVILEGES`, `ERR_INFRA_QUOTA_EXCEEDED` or `ERR_INFRA_DEPENDENCIES` then it is now immediately set to the `Failed` status (this already happens also for `ERR_INFRA_UNAUTHORIZED` or `ERR_CONFIGURATION_PROBLEM`). This prevents Gardener from automatically retrying the operation. If you are hit by it, please manually retry the operation once you have resolved the issue.

/assign @danielfoehrKn


What is the best practice to handle parent-child pair whose relative movement is in multiple dimensions?

Thanks for releasing the nice code.
In https://github.com/KevinLTT/video2bvh/blob/312d18f53bf31c37adcaf07c97098b67dbf9804a/bvh_skeleton/coco_skeleton.py#L32
we can see that Nose has two children: LeftEye and RightEye
'Nose': ['LeftEye', 'RightEye'],
The initial direction of LeftEye seems to have two dimensions of movement, along x and z?
What is the best practice to handle this case? Should we introduce a pseudo joint MidEye?

I think interpolate a 'MidEye' with 'LeftEye' and 'RightEye' is OK, if you want use them to calculate the rotation of 'Nose'.

Hi, @KevinLTT
How do you get this initial direction?


Add documentation

Please

Explain how each protection works and the corner cases
Explain how to correctly implement each protection in a Rack/Sinatra app
Explain how to test each protection in a live app (to ensure they are properly enabled)

I would agree there is serious lack of examples, but the docs should be accessible now as of closing #161.
I'll close this now and open up a new ticket upstream to explain the situation -- which hopefully will gain more traction than this repo.


Feature/add validation endpoints 171

Did you mean to remove the tileserver file?

Isn't the tileserver lakes.mbtiles file in the incorrect place? It should be generated having run the scripts instead of being present in the codebase from the start

Isn't the tileserver lakes.mbtiles file in the incorrect place? It should be generated having run the scripts instead of being present in the codebase from the start

Lakes is the only one that should be checked in - the process of calling the Open Street Map API to get about 300MB of lake data is very slow for what is actually a small output file. It's a trade off rather than having to wait for it to be built. This is correct currently.


Set up beta/training/demo environment

Stand up additional environment for beta/training/demo
CURRENTLY USING TEST ENVIRONMENT

Needed for CAMPD Beta Release. Adding to PI 6 and MP Beta Release


Support skipping region validation

What problem are you facing?
Terraform provider-aws supports the field skip_region_validation to enable the use of custom regions in local deployments such as Localstack and Minio, but this is not supported in upbound/provider-aws

How could Official AWS Provider help solve your problem?

Enable the skip_region_validation field via the AWS ProviderConfig e.g.
apiVersion: aws.upbound.io/v1beta1
kind: ProviderConfig
metadata:
  name: local
spec:
  ...
  skip_region_validation: true
```

PR opened by @carpenterm: https://github.com/upbound/provider-aws/pull/814


Using repositories from Github Legacy Search as scope for Github Code Analyzer

I need to use repositories discovered by Github Legacy Search as input for Github Code Analyzer. I am unable to find any documentation within the Wiki about how I could do this.

Hi @rahulsom ,
Github Code Analyzer runs on Github projects specified by an org or user name.  Currently there isn't a way to send that specific analyzer an existing Result Filter as input for analysis.
If you could explain a little more about your use case I can see if we have other task types that would help.
-Scott

Hey @sbehrens ,
I'm looking for certain keyword matches across public Github repositories. My initial thought was to use legacy search but legacy search by default just returns a link to repositories matching the keywords. Next, I tried using Github Legacy search with search type parameter set to 'code', but the task would complete without producing any results (Possibly due to too many matches? I couldn't find a way to limit search results within Legacy Search). I then attempted to use the repository links populated in results by Github legacy search as input for Github Code Analyzer which is what caused me to post this question!
To summarize: I want results to be populated with code from public repositories that contain certain keywords. I cannot use Github Code Analyzer since I do not know which repositories / organization / users, I want to sync. Rather, I need to do this dynamically as I get new matches.
I can do this function using the Github UI - Example
Thanks,
Ron

Hi Ron,
This sounds like a really cool use case that the tool currently doesn't support.  I currently don't think we'll throw this on our personal roadmap due to other commitments.  I'll put a note that we want help to build this out, and hopefully, someone in the community can dedicate a little time to make this happen.


Use async as a suffix rather than a prefix for asynchronous variants.

This is better aligned with standard javascript convention.
Often libraries have the default version be asynchronous, and
name the Sync one explicitly, we are marking the async ones as
they are by far the most common and usable for pipeline
construction.

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[ ] Choose reviewer(s) and mention them in a comment (R: @username).
[ ] Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
[ ] Update CHANGES.md with noteworthy changes.
[ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

R: @pskevin

I was wondering if we should default to making all apply calls be Async (they would return some sort of Future) and we could have futures applied on futures... wdyt Robert?
This is of course a big change to the API - I'm just spitballing...

I did consider this, but it would make chaining much more verbose. E.g. instead of
lines
    .map((s: string) => s.toLowerCase())
    .flatMap(function* splitWords(line: string) {
      yield* line.split(/[^a-z]+/);
    })
    .apply(beam.countPerElement())

one would have to write
(await
    (await
        (await lines
            .map((s: string) => s.toLowerCase())
        ).flatMap(function* splitWords(line: string) {
      yield* line.split(/[^a-z]+/);
    })
    ).apply(beam.countPerElement()))

or
lines
   .map((s: string) => s.toLowerCase())
   .then((result) => {
      return result.flatMap(function* splitWords(line: string) {
        yield* line.split(/[^a-z]+/);
      })
    })
    .then((result) => {
      return result.apply(beam.countPerElement());
    })

which gets really painful. (Note that the future here is Javascript's Promise, which is a kind of special builtin).

I thought of something like this:
chainedPromises = lines
  .map((s: string) => s.toLowerCase())
  .flatMap(function* splitWords(line: string) {
        yield* line.split(/[^a-z]+/);
      })
    .apply(beam.countPerElement());

countPcoll = chainedPromises.resolve();

// In case of error while expanding an intermediate PCollection, then we can throw/return an error from there when resolving the PCollection
// (error e.g. to resolve schema, or to match input/output types / etc).

Where each apply(...) returns a Promise<PCollection<...>> and the actual expansion is deferred...
though I haven't researched JS enough, I think this is doable...

To do this one would have to add methods like flatMap to the Promise prototype (affecting all Promises across Javascript) or return a subclass (well, really a hierarchy of subclasses) of Promise (which I looked into and looks pretty sketchy). This would work if Promise could be a proxy object to the type it was promising, but it's not.
It looks like another alternative is
lines
    .invoke("map", (s: string) => s.toLowerCase())
    .invoke("flatMap", function* splitWords(line: string) {
      yield* line.split(/[^a-z]+/);
    })
    .invoke("apply", beam.countPerElement())

but this is still a bit icky, and looses type safety.

Well thanks for doing the research and sharing your thoughts:)
On Mon, Jul 11, 2022, 12:45 PM Robert Bradshaw @.***>
wrote:

To do this one would have to add methods like flatMap to the Promise
prototype (affecting all Promises across Javascript) or return a subclass
(well, really a hierarchy of subclasses) of Promise (which I looked into
and looks pretty sketchy). This would work if Promise could be a proxy
object to the type it was promising, but it's not.
It looks like another alternative is
lines
.invoke("map", (s: string) => s.toLowerCase())
.invoke("flatMap", function* splitWords(line: string) {
yield* line.split(/[^a-z]+/);
})
.invoke("apply", beam.countPerElement())
but this is still a bit icky, and looses type safety.
—
Reply to this email directly, view it on GitHub
https://github.com/apache/beam/pull/22134#issuecomment-1180797546, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAJ5Z3FKBOP5NE5BJBACZBLVTR2WLANCNFSM52N5F5TA
.
You are receiving this because you commented.Message ID:
@.***>


Azure typescript e2e apps react app sas token vite

Purpose

...

Does this introduce a breaking change?

[ ] Yes
[ ] No

Pull Request Type
What kind of change does this Pull Request introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

Get the code

git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install

Test the code

What to Check
Verify that the following are valid

...

Other Information

Closing due to issue with windows/fn pm 4.


There is a problem in torchvision.datasets.Kinetics

🐛 Describe the bug
I’m trying to use torchvision.datasets.Kinetics but I’m getting this Assertion Error.
This is the initialization of the dataset
test_dataset = torchvision.datasets.Kinetics(root=root_path, frames_per_clip=33,
     step_between_clips=5, num_workers = 40, split='test', transform = transforms.RandomResizedCrop(size=224))

This is error I get:
AssertionError: Caught AssertionError in DataLoader worker process 0.
Original Traceback (most recent call last):
  File "/home/bairouk/anaconda3/envs/deep/lib/python3.10/site-packages/torch/utils/data/_utils/worker.py", line 287, in _worker_loop
    data = fetcher.fetch(index)
  File "/home/bairouk/anaconda3/envs/deep/lib/python3.10/site-packages/torch/utils/data/_utils/fetch.py", line 49, in fetch
    data = [self.dataset[idx] for idx in possibly_batched_index]
  File "/home/bairouk/anaconda3/envs/deep/lib/python3.10/site-packages/torch/utils/data/_utils/fetch.py", line 49, in <listcomp>
    data = [self.dataset[idx] for idx in possibly_batched_index]
  File "/home/bairouk/anaconda3/envs/deep/lib/python3.10/site-packages/torchvision/datasets/kinetics.py", line 234, in __getitem__
    video, audio, info, video_idx = self.video_clips.get_clip(idx)
  File "/home/bairouk/anaconda3/envs/deep/lib/python3.10/site-packages/torchvision/datasets/video_utils.py", line 373, in get_clip
    assert len(video) == self.num_frames, f"{video.shape} x {self.num_frames}"
AssertionError: torch.Size([34, 720, 1280, 3]) x 33

Thank you.
Versions
pytorch                   1.11.0
torchvision               0.12.0
python                    3.10.4

That seems more related to the
https://github.com/pytorch/vision/blob/7d36d263a8356fac0bb363617b0c57c3bac6f89f/torchvision/datasets/video_utils.py#L73
object than the dataset itself. @YosuaMichael / @jdsgomes / @bjuncek could one of you have a look?

@DaBihy I tried to build the same kinetics test dataset and iterate some samples, but I can't reproduce the error.
Do you know which exact video that cause the problem? This would help us to check more detail and try to reproduce the error.

@YosuaMichael This is some of the videos that cause the error. they work just fine when I play them on a media player.
yaoNwz99xM_000062_000072.mp4
3caPS4FHFF8_000036_000046.mp4
0wR5jVB-WPk_000417_000427.mp4
3yaoNwz99xM_000062_000072.mp4
0wR5jVB-WPk_000417_000427.mp4
Thank you.

Thanks for sharing the video @DaBihy , I confirm I can reproduce the same error on those video with torchvision 0.12.0.
When I run the same script on latest torchvision 0.13.0, the error gone, so I guess this probably has been fixed.
@DaBihy I suggest you to update the torchvision version and see if that fix the issue.
I think this should fix the issue so I will close this, but if you still get the problem, feel free to reopen this issue.

@YosuaMichael  Thank you for the rapid answer. the latest version does not have the bug.
On the other hand torchvision.datasets.Kinetics doesn't have a function to resize audio which causes the following error RuntimeError: stack expects each tensor to be equal size, but got [2, 31744] at entry 0 and [2, 32149] at entry 1. It's normal to have such an error because default_collate can't stack audio with different sizes. Anyway, in my case, I don't need audio so I eliminated it using a custom ```collate_fn``. It would be great if you added a function for audio transform
thank you again @YosuaMichael @pmeier

It would be great if you added a function for audio transform thank you again

We won't given that this is out of scope for a vision library that we are. If you need the audio for something, you should have a look at torchaudio that specializes in these things.

Anyway, in my case, I don't need audio so I eliminated it using a custom collate_fn.

That is what we also do:
https://github.com/pytorch/vision/blob/f467349ce0d41c23695538add22f6fec5a30ece4/references/video_classification/train.py#L129-L132
(d[1] is the audio)


How can I add select all on responsive vertical menu? Is there a property to it

Thank You! I solved it using customSelectToolbar property


FirebaseUI/Auth with Email-Password

I'm hoping to use FirebaseUI/Auth with Facebook, Google, and email/password authentication options.
On the FirebaseUI/Auth landing page, it shows how to add Google and Facebook providers:
FIRGoogleAuthUI *googleAuthUI = [[FIRGoogleAuthUI alloc] initWithClientID:kGoogleClientID];
FIRFacebookAuthUI *facebookAuthUI = [[FIRFacebookAuthUI alloc] initWithAppID:kFacebookAppID];
authUI.providers = @[ googleAuthUI, facebookAuthUI];

How would I add email-password to the auth providers option and have it appear on authViewController?

Email-password is enabled as a sign-in option by default, it doesn't require any configuration :)
A "Sign in with email" option should appear on authViewController, unless signInWithEmailHidden is set to YES.

Awesome!
Just wondering, where can I find the properties of <authViewController>?

The authViewController is essentially a UINavigationController thats wraps a FIRAuthPickerViewController, there's not much you can do with the UINavigationController itself with the current API.
However, if you want to customize the UI of the first screen, you can provide an instance of a custom subclass of FIRAuthPickerViewController by implementing the FIRAuthUI delegate method authPickerViewControllerForAuthUI:. In your custom subclass, you can modify the default UI as you want.

Closing this issue now. Feel free to let us know if you have more questions :)

In version 0.5.0 on uidemo there is no e-mail field, and setting signInWithEmailHidden to false doesn't help

I can confirm, if you don't set a provider you just get a blank view controller with a title of 'welcome'

For everyone having this problem on 5.0:
https://stackoverflow.com/a/54608189


from where i get the id ads?

hi where i can find id ads like admob id adverist ca-app/xxxxxxxxxxx-xxxxxxxx
because i need change them with real ads not test ads ?? give me the url please and thank you

Go to https://developer.huawei.com/consumer/en/monetize/ and click "Start now", then go to HUAWEI Ads Publisher Console
https://developer.huawei.com/consumer/en/service/ads/publisher/html/index.html?lang=en
apply real ad unit id.


Replace Div with attribute, insert Razor section

Hi, this is great!! 💯  I am trying your scripting using the sample where I need to replace many items for print depending on the machine etc
can you please share a sample on can we find div with attributes? and replace the tags with a section - while keeping the same text or html - content on the inside.
find this

print stuff only

  insert this with same old content -> @section print {
 print stuff only
 }

query and replace DIV's containing Class="Print Media"
and then replace with Razor section like @section print {}

thanks

Use the standard QuerySelectorAll (for help on this consult any guide for web development) and the replaceChild method.
I am not sure I get your question. Razor is something different. Do you want to introduce strings here, print the serialized code and save this as a cshtml file?


Add concat fusions

This will fuse the inputs to concat if they are pointwise operators.

Codecov Report
Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (2cd77b9) 91.38% compared to head (a952876) 91.41%.
 Consider uploading reports for the commit 3f0fed0 to get more accurate results

Files
Patch %
Lines

src/cpp_generator.cpp
75.00%
2 Missing :warning:

src/fuse_concat.cpp
97.40%
2 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2460      +/-   ##
===========================================
+ Coverage    91.38%   91.41%   +0.03%
===========================================
  Files          454      456       +2
  Lines        17189    17275      +86
===========================================
+ Hits         15708    15792      +84
- Misses        1481     1483       +2

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Fix CI but otherwise I get the idea.


Block EC module clean up

For example, we can remove the raid source folder for now. It can be kept in a branch though.

And also, not so relevant deps in the module pom.xml file.

Solved.


Change template engine to Razor Engine

Wanted to confirm, it looks like you want your Email template handling to be done through Razor views. Is that correct? If so I would like to take a crack at it.

@lira92 how complex is this?

Razor engine is a part of Asp Net MVC, but this lib https://github.com/Antaris/RazorEngine delivery the engine only, in this file https://github.com/Inside-Sistemas/mailer.net/blob/master/Mailer.NET/Mailer/Rendering/EmailRender.cs is the template render logic, that is implemented with NVeloicity, an engine that is deprecated.

I'd be happy to add razor templates for the HTML files.
Shall i convert one of the HTMLs to CSHTML (with razor) and raise a PR so you can take a look?

Oops! Looks like a PR has already been raised for this :) let me know if you need any further work

@raghav710 I haven't seen either, rsrs, thanks


Handles SDK package rename; file -> files

Replaces #14

Failures are expected:

Response : test_signature_request_create_embedded {'error': {'error_msg': 'Must specify file(s) to be sent.', 'error_path': 'file', 'error_name': 'bad_request'}}

For C#, errors are stderr=b"Unable to find image 'hellosign/dropbox-sign-dotnet:latest' locally\ndocker: Error response from daemon: pull access denied for hellosign/dropbox-sign-dotnet, repository does not exist or may require 'docker login': denied: requested access to the resource is denied.\nSee 'docker run --help'.\n")
For Java, errors are stderr=b'Error: Could not find or load main class com.dropbox.sign.sdk.Requester\nCaused by: java.lang.ClassNotFoundException: com.dropbox.sign.sdk.Requester\n')
For Python, those two account related tests failed due to dropbox_sign.exceptions.ApiTypeError: Invalid type for variable \'account_create_request\'. Required value type is AccountCreateRequest and passed type was AccountCreateResponse at [\'account_create_request\']\n')
The detailed error messages are hard to spot. You will need to scroll up the log quite a bit


_AMM_ModScript_Manager defaults to CombinedModType 3

Since yesterdays update _AMM_ModScript_Manager defaults to naming combined mods COMPOSITE-NAME COMBINED MOD PAK like Mod1+Mod2+Mod3.pak instead of asking.
It also ignores CombinedModType setting in BUILDMOD_AUTO.bat

hmm, I guess you mean AMUMSS, because AMM does not create mod paks.
What version of AMUMSS?
When using AMM, is Auto checked?

if Yes, you are using BUILMOD_AUTO.bat, CombinedModType 3 (create a COMPOSITE name)
if No, you are using BUILDMOD.bat which default to CombinedModType ASK

I do not see the behavior you are describing, if I understand correctly.
I would suggest you come to amumss-lua Discord channel
It would be easier to help you in real time...
I believe this is a valid invite: Invite

I've been using ModScriptManager to start the batch jobs.

using BUILDMOD_AUTO.bat -CombinedModType 2 is ignored
using BUILDMOD.bat it doesn't ask
only happens since the most recent update to the manager that also added all those neon colors.

Beside that AMM is v<IP_ADDRESS>, you did not give me more info...
I cannot duplicate your problem as my AMUMSS <IP_ADDRESS> install does not exhibit that behavior and works correctly.
AGAIN, please come to Discord if you want this resolved in a timely manner, otherwise it will take forever for us to exchange info in this issue
I would suggest you come to amumss-lua Discord channel
It would be easier to help you in real time...
I believe this is a valid invite: Invite

Will do, thanks.

I am there at the moment


Swipe issue

When we swipe the sheet little bit, then try to tap in the  overlay to dismiss, that is not happening in ios 13

Facing the very same issue.

its lead to frozing the app for me

Karthik this problem got automatically resolved when I updated the device version to beta iOS 13.1(17A5821e) and Xcode to version 11 beta 7

Same issue here. It's very frustrating because it essentially bricks the app. Would love a fix asap, so we don't have to pull the library from our app.

@mattbanach Any thoughts on this? I've dug around, but haven't pinpointed the root of this. Any chance this is introduced with changes to fix iOS 13 in #20 ? Any help here would be appreciated.

Facing the same issue in earlier version too

Hi there, what we have done (as a temporary fix) is
"finish()" the animation on swiping down instead of "cancel()"
In my version I had to do it on line 105 of PresentationManager.swift file.

attached is a screenshot.


docs: add note about chainsaw

Description of the change:
Add note about chainsaw in the docs.
If you need more infos :point_down:

GitHub: https://github.com/kyverno/chainsaw
Docs: https://kyverno.github.io/chainsaw
Slack: https://kubernetes.slack.com/archives/C067LUFL43U

Motivation for the change:
We presented chainsaw to the community meeting last week and received good feedback.
It would be awesome to have it listed in the docs.
Checklist
If the pull request includes user-facing changes, extra documentation is required:

[ ] Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
[ ] Add or update relevant sections of the docs website in website/content/en/docs

Following the discussion we had via Slack, I wanted to share some thoughts on the PR.
The operator-sdk includes a feature called scorecard, which utilizes kuttl. This is the reason kuttl is mentioned in the documentation. However, chainsaw is not included or shipped with the project. Therefore, I'm not sure if it's appropriate to include it in the documentation.
Including chainsaw might lead the operator-sdk maintainers into a situation where they have to answer questions and provide support for a tool that is not a direct part of the project. It can be challenging for maintainers to support third-party tools and integrations.
I am no longer an active maintainer of the SDK project, so I'll leave the decision to those currently maintaining the project to determine what best fits their strategy.

Thanks for chiming in @camilamacedo86 !
Operator SDK can support chainsaw scorecard test, here is the PR https://github.com/operator-framework/operator-sdk/pull/6676 😄
I'm fine with answering chainsaw related questions if any.

@jberkhahn do you have any suggestion after the yesterday meeting ? what is the next step ?

Thanks @varshaprasad96 😍
Do I need to fix something ?

@varshaprasad96 i lost your LGTM 🙏

Going to go ahead and merge this, as it has been open for review for a few days! If folks have any issues/further discussions w.r.t to this PR, please feel free to create issues :)


When should we load the data about projects?

We can load all the data during login, but we need to logout and login to update our information and
we will duplicate that information in our db.

I think we should load data about projects directly after login - and data about projects stored in database should be always the newest ones (without duplicates).
My proposal:

Refresh projects list button (to manually trigger refreshing list of projects)
Load projects list directly after login

Pimp my Pr class diagram
https://www.draw.io/#G1eUzbNT2zW3CJf0BSit48jdhto3qVKfP9


How do you pronounce sake?

As in "for Pete's sake," or as in the alcoholic beverage of fermented rice?

I want to know this. You should add it to the readme

Haven't decided yet, initially it was meant as a play on words with regards to make (s for server), so it would be for Pete's sake in that case. But then again, I had in mind it could be as in the fermented rice drink.

With great power comes great responsibility.  It's up to you now, @alajmo.


don't cancel all other actions on non-master

it appears that whenever the master filter action exits neutrally on non-master, all other actions are cancelled.
This is probably intended behavior and just means that I need to add another workflow.
Still, should investigate + document.

https://github.community/t5/GitHub-API-Development-and/Cancelled-Github-actions-fails-PR-checks/m-p/15998#M440
https://github.com/actions/bin/issues/13

just to remind myself; it's not clear that this is actually a limitation or problem; I think I had just written wrong dependencies.
If you filter as late as possible, then this works out just fine.


Missing ReferenceAssemblyAttribute decoration

The attribute ReferenceAssemblyAttribute informs analysis tools that the assembly is not a runtime assembly.
Also consider decorating with AssemblyFlags((AssemblyNameFlags)0x70) to prevent the runtime from actually loading the assembly.  Example in AspNetCore.

My tests actually load the reference assembly and use reflection to inspect it, so I need that part of it. And at least MacOS doesn't support ReferenceOnly loading. I'm afraid I have to leave this as it is. The nuget package should be keeping the SMA.dll out of publish locations.

My tests actually load the reference assembly and use reflection to inspect it, so I need that part of it.

The suggested code could be under #if-#else-#end condition.

Yeah @JamesWTruher you should be able to add the ReferenceAssemblyAttribute decoration without changing anything, afaik it's just the ProcessorArchitecture=None that disables loading the assembly.
I do think it's important to include the assembly flag though.  One of the most common problems I see folks run into is trying to or accidently using this assembly in xunit tests.  I've seen folks troubleshoot mysterious NRE's for actual days assuming it's a problem with their tests.  Unless you're already familiar with how reference assemblies are built, it's incredibly difficult to determine what's going on. An assembly load time exception would make tracking down the issue way easier.
I think @iSazonov's idea is best value so to speak, in that it's super easy to get up and running.  You can just set up a build time constant that you pass to dotnet when building for tests.  If you wanted to go the extra mile you could set up the tests to use MetadataReader.  That API is a lot more complicated and scarcely documented, but if you go that route you can use this as reference.  It implements all the System.Reflection API's using SRM so it can be used as a reference on how to translate existing reflection code.

i experimented with this, when the attribution was present, the module could not be loaded and my tests cannot run. On Mac, a message in produced which says that reflection only loading is not supported on the platform. If you have a different experience on non-Windows, I would love to know how. @iSazonov, @SeeminglyScience

Oh hey look at that it does, I wonder what the point of the ProcessorArchitecture=None decoration is then...
Anyway as noted above, you should be able to get your tests working pretty easily by defining a build time constant when building for tests and using preprocessor directives to exclude the attribute(s).

I guess I'm not clear. I'm using the shipping assembly and testing it in my Pester tests. The pester tests load the assembly and then runs the tests against it. I really don't want to build one assembly for test and then another for ship.

I hear ya, but I've helped quite a few folks troubleshoot things like "why is Parser.ParseInput returning null" just from occasionally looking at the PS slack. Not one of them considered that it could be the library they were loading, they were all trying to troubleshoot their tests.  Even @TylerLeonhardt ran into that iirc.
This would at least give those folks something concrete to google.

I found .Net Runtime uses the attribute too
https://github.com/dotnet/runtime/blob/master/src/libraries/Common/src/Extensions/ReferenceAssemblyInfo.cs
Perhaps there are tests for reference assemblies too as example.

@SeeminglyScience yeah we hit that trying to use PSStandard inside of xUnit tests... Obviously that wouldn't work until we loaded in the Microsoft.PowerShell.SDK instead of PSStandard.

On Mac, a message in produced which says that reflection only loading is not supported on the platform.

I found only https://github.com/dotnet/runtime/issues/31200. It is for ReflectionOnlyLoadFrom(). I wonder if MetadataLoadContext is not supported on Mac.

@SeeminglyScience yeah we hit that trying to use PSStandard inside of xUnit tests... Obviously that wouldn't work until we loaded in the Microsoft.PowerShell.SDK instead of PSStandard.

Yeah that's it.  And just to be clear, that's expected and won't change with this. The big difference is that with this change, if that same mistake is made you'll get an error message about loading a reference library.  The only problem here is the cryptic null reference exceptions that folks get that leads to days of troubleshooting tests because no one understands how reference assemblies work (nor should they need to).


pkg,cdc: remove actor system stop errors

What problem does this PR solve?

Issue Number: close #4887
Check List
Tests

Unit test

Code changes

Has exported function/method change

Release note
None

Codecov Report

Merging #4888 (79be6f2) into master (9607554) will increase coverage by 0.4040%.
The diff coverage is 69.8150%.

Flag
Coverage Δ

cdc
60.4698% <69.8150%> (+0.5475%)
:arrow_up:

dm
52.4083% <ø> (+0.3794%)
:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.
@@               Coverage Diff                @@
##             master      #4888        +/-   ##
================================================
+ Coverage   55.6402%   56.0442%   +0.4040%
================================================
  Files           494        523        +29
  Lines         61283      65500      +4217
================================================
+ Hits          34098      36709      +2611
- Misses        23750      25214      +1464
- Partials       3435       3577       +142

/merge

/run-leak-test


146a simplify training - but backwards compatible

This PR is a backwards-compatible alternative to #148. I think this could be a better approach. It opens up some flexibility, without sacrificing existing functionality. Specifically it accomplishes this by

converting the trainer class to a more generic train function.

I think a function is easier to use than a class and the extra functionality that a class provides isn't really needed in this case
This function can be used independent of the train_model function which I see as a wrapper around the train function that provides some additional structure
This makes it so a user can use the function with any arbitrary model and arbitrary file names for the outputs weight directories and log files.

Providing the option of passing a compiled model into train_model

In this way, if a user likes the extra structure of the train_model function, but they want to use their own custom model and they don't want to code it into the train_model function, they can just pass it in as the model argument and bypass the if else statements for defining and compiling the model.

providing the option of passing a compiled model to the predict functions

Similar to above, the allows the user to pass in their own compiled model and then they/we don't have to code it into the river-dl codebase.
The existing models (RGCN and LSTM) are still accessible via the model_type argument like before.

I would love your feedback, @janetrbarclay  and @SimonTopp. And it would be super helpful if you could pull this branch and see if it indeed works with your existing Snakefiles.

Hey @jsadler2, I'll try to get to reviewing this later today, but wanted to comment quickly on the impetus behind this PR over #146.  Specifically in you're last comment:

@janetrbarclay mentioned that it would be ideal if (paraphrasing here) if we can add the new functionality without losing the existing functionality. I think that is wise. I think there is a pretty straightforward way to do that. I will revise this PR to do that.

I'm not sure I agree that we should keep the option to pass in model_type over just passing a compiled model.  River-dl is already a large and complex workflow repository.  I'm of the mind that in simplifying the training routine we should also be simplifying the codebase.  Keeping functionality for both seems like it's adding unnecessary lines of code, and while it's more flexible, it also creates redundancy and requires additional explanation of the various ways you can use the repository.  For new users, and to maximize generalizability, I think it's more helpful to have a standard high level workflow that we can explain in detail in the Readme.  This seems like a high level conversation that might influence how we review the PR, so I wanted to throw it out there before @janetrbarclay or I got started. Thoughts?

My comment was intended more that we should be sure the code can achieve the same results (even if not quite the same way), not so much that we should have backward compatibility. I haven't thought much about the question of whether we need to maintain backward compatibility, though my inclination is that it might not be needed since we're a small group and our projects are at the point that we can adjust relatively easily. I do like the idea of streamlined code, especially if we provide sufficient examples (as we've already said we need) of how to set up the model.
That being said, I did pull this PR and ran it with both my base config/Snakefile and the GW loss config / Snakefile and it ran without error.

Okay. Thanks for clarifying, @janetrbarclay and thanks for your thoughts, @SimonTopp.
I agree with the sentiment of "let's simplify even at the expense of backwards-compatibility" that I think I'm hearing from both of you.
That being the case, I will close this PR and let's go back to the #146


Web ssl

Based on code / comments in https://github.com/locustio/locust/pull/278

I'm a little bummed this was closed - it's a pretty straightforward feature and I see merit in it. I wouldn't expect it to be SSL-enabled by default, but if someone has a key/cert and they want to serve the UI over https they can't do that? Or can't without setting up nginx or some other kind of reverse proxy?
I'd like to spin up a locust server in our private cluster and share the URL with my team. I'd feel a lot better sharing an https URL than an http one. There's no specific attack vector I'm worried about, moreso that it's 2020 and the web should be secure by default. Prohibiting this when it's just a 2 line code change seems a bit strange, but those are just my 2 cents.

I think we should consider accepting an updated PR for this, especially now that we've added Basic Auth support to the Web UI.

Agreed, thanks @heyman - took a crack at it here https://github.com/locustio/locust/pull/1354


Add default prompt for LLM

Description

Partially supersedes #2554 together with #2657.
Type of change

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update
[ ] Code quality improvements

Checklist

[ ] I have tested this code
[ ] I have updated the documentation

Blocked by the backend PR still being coupled together, even though they are two different features.


perf: Implement Clone without inline for some enums

Otherwise each crate which does a clone gets its own copy of the
implementation. This has been done for enums which were identified
as causing bloat in the output binary. This reduces the binary size
and shouldn't negatively affect performance because these were too
large to be inlined anyway.
   text	   data	    bss	    dec	    hex	filename
-21101321	 520952	   2560	21624833	149f801	target/release/swc
+20233529	 520984	   2560	20757073	13cba51	target/release/swc

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

There's failures because I haven't updated ast_node usage for the whole project. If this PR is something you want, then I can fix those too.

I definitely want it, thank you!

Maybe we could use an argument to ast_node to avoid the breaking change?

Oh, yeah it would be nice


As-Is/To-Be preview added with latest changes

Content looks good, though you seem to have inadvertently changed the indentation of GenericList::Render


[FLINK-18110][fs-connector] StreamingFileSink notifies for buckets detected to be inactive on restoring

What is the purpose of the change
This PR fixed the issue that the BucketLifeCycleListener in StreamingFileSink misses the notification for buckets detected inactive on restoring. On restoring, the buckets with only pending files will get all the pending files committed and will not be recorded. At this time, StreamingFileSink should notify that the buckets get inactive.
Brief change log

18db5ae4db2c5284bf0530c60e05f02be3a40d19 adds the missed notification.

Verifying this change
This change added tests and can be verified as follows:

Added test that validates that the notification is sent on restoring if buckets get inactive.

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): no
The public API, i.e., is any changed class annotated with @Public(Evolving): no
The serializers: no
The runtime per-record code paths (performance sensitive): no
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn/Mesos, ZooKeeper: no
The S3 file system connector: no

Documentation

Does this pull request introduce a new feature? no
If yes, how is the feature documented? not applicable

Very thanks @JingsongLi  for the reviewing! I have update the PR according to the comments.


Cleaned readinessProbe test and binary Make targets.

Signed-off-by: Renan Campos<EMAIL_ADDRESS>

@JohnStrunk please merge


Pages not automatically appearing when created while npm run dev running

Environment

Operating System: Darwin
Node Version:     v14.19.1
Nuxt Version:     3.0.0-rc.4
Package Manager<EMAIL_ADDRESS>Builder:          vite
User Config:      -
Runtime Modules:  -
Build Modules:    -

Reproduction
npx nuxi init nuxt-app
cd nuxt-app
npm install
npm run dev -- -o

Add a new page into /pages folder (eg. test.vue)
In your browser visit: http://localhost:3000/test
Get a 404
Stop/Start npm run dev -- -o
In your browser visit: http://localhost:3000/test
Get the page!
Describe the bug
Any files added to /pages folder while development server is running should be reachable in the browser straight away. Instead you have to restart nuxt.
Additional context
No response
Logs
No response

any updates?


Import UI for Pinboard

I think this is blocked by having a job queue https://github.com/lmorchard/pebbling-club/issues/56


Improve provenance logging

Many ideas for ways in which the ESMValTool provenance recording can be improved are listed in the pull request with the original implementation, https://github.com/ESMValGroup/ESMValTool/pull/649. When there is time, it would be good to try to implement some of those.
A good provenance viewer that can zoom/collapse details would also be much desired, to make it possible for a human to view the recorded provenance information. Until we have a good viewer, adding (much) more detail may not make that much sense, because provenance is mostly meant to be read by humans, though specific details may be extracted in an automated way, e.g. for display on websites.

Suggestion by @axel-lauer: also record provenance for files from auxiliary_data_dir.
It would also be good to issue a warning or raise an error if there is a mistake in diagnostic_provenance.yml (e.g. trying to record a file as ancestor that is not a direct ancestor), in addition to #885.

Another idea for improvement: record provenance per panel in multi-panel plots.


[CosmosDB] Renaming all references to autopilot

Latest improvements:
MSFT employees can try out our new experience at OpenAPI Hub  - one location for using our validation tools and finding your workflow.

Contribution checklist:

[x] I have reviewed the documentation for the workflow.
[x] Validation tools were run on swagger spec(s) and have all been fixed in this PR.
[x] The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

[ ] Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
adding/removing APIs.
adding/removing properties.
adding/removing API-version.
adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

[ ] If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Please follow the link to find more details on API review process.

/azp run automation - sdk

/azp run automation - sdk

/azp run automation - sdk

/azp run automation - sdk

/azp run automation - sdk

Can one of the admins verify this patch?

@MyronFanQiu / @markcowl / @yungezz can someone review this PR?

@ravgill although ARM signed off, this PR will cause SDK breaking change since renaming properties in existing stable version.

@yungezz Yes, just found out that it breaked Fluent .NET SDK :-(


Implement YouTube search

Working on this on, check my new brach

So it just spawns URLs into the chat.
But what if there was a way to just click the appropriate video’s title to download it? We could remove URLs from the print AND increase UX!
You should try and do that, @piunov1998.

And how do u see it?

I just started
I cant make clickable elements for bot.
Im going to add some subcommand which will execute download function with chosen video url as argument

Check new update


fix: install libreoffice 7 apk from s3

Summary
S3 location with APKs: https://utic-public-cf.s3.amazonaws.com

[x] remove downloaded APKs after installation which were no longer needed, this alone reduced the image size by 1.2 GB ❗
[x] replace official libreoffice package with our custom one (built using melange)
[x] also test libreoffice 24 -> 🔴 does not work with .ppt files!

LGTM assuming tests pass. We were hitting the test_dockerfile failure in the unstructured repo as well and I think it may be due to a new libreoffice version getting pushed out to the wolfi package manager.

our libreoffice 7 build works!


Rendering of literal star characters in documentation

In reading your documentation, specifically that surrounding wildcard hostnames for a PR on Traefik, I noticed that you appear to be suffering from markdown rendering literal * characters as markup generating  tags. On slack I was shown that this had indeed been noticed but the solution wasn't obvious to you. I believe if it is markdown, that wrapping the offending wildcard hostnames in backticks should be a higher precedence operation and prevent it from interpreting them as emphasis marks.
Per our discussion on slack I'm opening this bug report for tracking.

/assign

I did some debugging here and here's the least bad solution I could find:
`\*`

Simply using backticks without the \ would result in the markdown appropriately getting * instead of . So that solves an issue with gen-crd-api-reference-docs. Unfortunately we immediately turn around and try to convert that to html with mkdocs, and that predictably converts * to . Adding \ helped everything get past that step, but I think even this is probably a step too far since it makes the godocs rather confusing. Of course * is also a solution that gets around this, but I think that also results in godocs that are too confusing.
Not sure what the best path forward is here, I don't mind the switch to backticks but I can't find a great way to workaround this once we get to mkdocs. Maybe a simple sed command as part of make docs would work as a manual override?
/cc @jpeach

Simply using backticks without the \ would result in the markdown appropriately getting * instead of . So that solves an issue with gen-crd-api-reference-docs. Unfortunately we immediately turn around and try to convert that to html with mkdocs, and that predictably converts * to . Adding \ helped everything get past that step, but I think even this is probably a step too far since it makes the godocs rather confusing.

I'm surprised that a doc renderer coming from html would have that problem. The backticks should render as either a pre or a code tag (can't remember which) but it should skip either

I think part of the problem here is that gen-crd-api-reference-docs is generating this:
*

mkdocs translates that into

On the other hand, if we manually modify the generated markdown in doc-src to this:
`*`

Running mkdocs on that results in the appropriate output:
`*`

I guess I'm just saying that blindly double converting from markdown seems unlikely to succeed in other cases too. Can mkdocs be taught to ignore the content of code tags?

Actually, yeah, that's the problem. gen-crd-api-reference-docs is generating HTML and we're putting it in a md file so we can include it in our generated docs easily. We probably just need a better way to include that HTML.

On 12 Feb 2021, at 10:38 am, Rob Scott<EMAIL_ADDRESS>wrote:
Actually, yeah, that's the problem. gen-crd-api-reference-docs is generating HTML and we're putting it in a md file so we can include it in our generated docs easily. We probably just need a better way to include that HTML.

Rename the .md to .html?


Don't calculate the length of a constexpr string at runtime

It isn't required, but AFAIK every supported compiler will inline the call to strlen on a compile-time constant string.   Do you have evidence to the contrary?

No, I don't have.
I think it's a good idea to use "constexpr" to its fullest.

On the one hand, yes, constexpr is a good thing when available. On the other hand, it's already being done that way implicitly.  (Admittedly, without enforcement.)
On net, I'd rather not make the code harder to read for no technical benefit.  If any of our supported compilers were failing to inline that strlen call and it seemed like it mattered, I'd gladly go down this path.  For now, I'd rather keep it simple.


TypeError: must be str, not NoneType

$ ./make.py
Traceback (most recent call last):
File "./make.py", line 253, in
make()
File "./make.py", line 228, in make
" && make installer INSTALL_ROOT="+installdir + " IFW_PATH="+ifdir):
TypeError: must be str, not NoneType

Mind sending a PR to fix the issue?


Do Not Home on Boot

As a user, I do not want the robot to home automatically on boot.
Acceptance Criteria

Robot does not home automatically when robot turned on

Dependencies

Run App: Home Button #856

Closed by #1259


use utf-8 string encoding

Allow more characters to be used in presenceData (like é, ü, â, etc...) by using utf-8 encoding.

Thanks, I'll add some tests and try to get a new version out as soon as possible.

Released in v1.0.4, thanks a lot for the PR.

Thanks!
On Fri, Feb 13, 2015 at 5:16 PM, Paweł Ledwoń<EMAIL_ADDRESS>wrote:

Released in v1.0.4, thanks a lot for the PR.
—
Reply to this email directly or view it on GitHub
https://github.com/pusher/pusher-rest-node/pull/7#issuecomment-74279240.


Typo in live-share/docs/reference insiders.md

There is a typo in the docs
What was your system configuration?
Product and Version [VS/VSCode]: VSCode
OS Version[macOS/Windows]: macOS
Live Share Extension Version: v1.0.765
Target Platform or Language [e.g. Node.js]: n/a
Steps to Reproduce / Scenario:
On insiders.md there is a line that says, "Once such a session in planned in advance, you're able access it from the Live Share viewlet directly." However, the line should say, "Once such a session in planned in advance, you're able *to* access it from the Live Share viewlet directly."

#4897


Remote Lesson 8, Solution/Lesson Bug: Local-Main Moves In Solution, But Not In Lesson.

Sorry. New to Git/GitHub. I'll try to give as much detail as possible. I'm 99% sure this is a bug, but might be doing something wrong.
If you follow the solution to remote, lesson 8 "Locked Main" it goes like so:
git checkout -b feature C2   // Creates a feature branch at commit 2
git push origin feature         // Pushes the feature branch to remote, tracks origin feature branch at C2 in local branch
In the solution this also moves local-main to C1. When actually performed by the user, it leaves local-main at C2.

Actually, the solution has three commands.  And if you loss the first command, the level won't show solved.
git reset --hard o/main
git checkout -b feature C2
git push origin feature

Actually, the solution has three commands. And if you loss the first command, the level won't show solved.
git reset --hard o/main
git checkout -b feature C2
git push origin feature

Oh, so that's not just part of resetting the terminal to show the solution? That is part of the solution?
Thanks for clearing that up.

Actually, the solution has three commands. And if you loss the first command, the level won't show solved.
git reset --hard o/main
git checkout -b feature C2
git push origin feature

Oh, so that's not just part of resetting the terminal to show the solution? That is part of the solution?
Thanks for clearing that up. I think the confusion is that there is a message right after it, so it looks like that's part of it prepping for a solution and explaining why you might use --hard.
Edit: I'll close the issue once I get confirmation that was my issue. But it does seem that it was not in fact a bug (the 1% strikes the noob, haha).

Hi, if you look at the terminal, you can find one command before the solution is reset  --forSolution, that's the part of resetting the terminal to show the solution. I think you can confirm this by use show solution and observe the terminal in other levels.
Yeah, I think you are right, maybe the message confused and mistooken you to think the first command in solution is for resetting the level.  😊


feat(operator): add find, findIndex operator

add find, findIndex operator
add SingleValueSubscriber for common behavior to notify and complete
immediately

Continue effort to add operator exists in current RxJS.
find, findIndex operator is added.
In document of findIndex, return value is undefined if there isn't matching element

... defined by the specified predicate, if found; otherwise, undefined.

while example, and actual code returns -1 instead. This PR follows existing code behaviors to return -1.
add SingleValueSubscriber as base of FindSubscriber and others
One of common pattern of subscriber is let subscriber notify and complete immediately, usually related with operators using predicate like find, findIndex, isEmpty, single, last, and others not yet implemented. SingleValueSubscriber exposes notifyComplete(value) interface for those behavior to reduce duplication of code.

Performance implication? : Micro perf test for operators having inherited subscriber does not shows noticeable performance hits.
behavior of notifyComplete is quite short and small, is it really required? : Thinking about numbers of operators having similar behavior, I think it still makes sense to have common base. It still brings concern of inheritance depth usually want to avoid in general.

while example, and actual code returns -1 instead. This PR follows existing code behaviors to return -1.

This is the correct behavior.

Actually, @kwonoj ... I'm going to halt this PR for some bikeshedding. Nevermind my previous comments.

 I was trying to delete my previous comments. I'd stop work on this for right now... I'm suspicious that these operators might not have a lot of value to the library.
I want to create an issue to discuss it.

I saw issue is created. I'll followup issue and let's close issue based on conclusion.
Thanks!

@kwonoj ... can you add these operators in the extended directory, and to the KitchenSink output only? (see elementAt).

Sure, will do. https://github.com/ReactiveX/RxJS/pull/404 this should be merged first, is my understanding correct?

@kwonoj that's correct... sorry, I thought I'd merged it already. It's merged now.

Also @kwonoj, you may want to rebase now that I'm done merging PRs for the morning. There's an ugly merge in there from the CoreOperators/KitchenSink refactor

find, findIndex now placed under extended
Subscriber inheritance removed

updated module import paths for test case
removed redundant changes to PR

Thanks for your hard work, @kwonoj. Much appreciated!


WiP - EFS Updates

This patch updates EFS to use the Login interface for the storage driver as well as calculate the full instance ID on the client-side. Additionally, the new configuration keys efs.endpoint and efs.endpointFormat are introduced to enable custom endpoint URLs outside the scope of *.amazonaws.com per this REX-Ray PR/issue.
A subsequent update will occur to add the new endpointFormat key to the EBS driver as well.
This is a WiP PR. I'd appreciate it if @codenrhoden, @mhrabovcin, and @kasisnu could all please take a look and share their thoughts.

I don't understand why is this driver adding API connection to EC2 service and not using it anywhere? If we want to reuse AWS service connection abstraction between different drivers I'd say it would be worth extracting this to a separate package and not have it in EFS driver.

@akutz what is the reason for introducing session caching on EFS package level?

Hi @mhrabovcin,
You made two comments to which I'm unable to reply directly above, so please find my responses here after I include your original notes for convenience:

I don't understand why is this driver adding API connection to EC2 service and not using it anywhere? If we want to reuse AWS service connection abstraction between different drivers I'd say it would be worth extracting this to a separate package and not have it in EFS driver.

So my original thought in adding the API connection was to use it to gather the default Security Group information in case it was omitted from the configuration file. However, after I originally saw your note and thought upon it I realized my initial idea was flawed as the place where the security group information is used makes the context not server-centric, but instance-centric. Therefore I intend to use raw HTTP GET commands via the executor and pass the default security group (if any) via the instance ID's fields property much like I now do with region and/or availabilityZone from the client/instance. I will be removing the API connection from the server code.

what is the reason for introducing session caching on EFS package level?

The session caching is only secondary to the purpose of having client instance-scoped connections instead of one based on the server's configuration/instance information. Otherwise the server could never be outside the same zone as the clients which it manages. The caching was simply part of the parcel as it was something I had added on the EBS side for efficiency.

Hi @mhrabovcin,
I retract my statement about removing the EC2 connection on the server-side. It's still required in order to map the security group name from the client to the ID required by the EFS CreateMountTarget operation. On the client side I can only grab the security group names via the special IP, not the SG IDs as is required by the EFS operation. Thus I need to do an EC2.DescribeSecruityGroups on the server side using creds in order to translate the security group name into its ID. All the more reason that the connection is based on client-side details.

Hi @mhrabovcin,
I've validated that the default security group additions work as I hoped. If you don't have an strong objections, I'd like to merge this.

Hey @akutz, thanks for explaining the reason for including ec2 API client. If I am reading the code correctly it means that if there is no security group specified in the driver configuration you'll try to guess the security group(s) from a instance.
I don't think this would work in usual way how security groups are structured. There is no reason to open NFS mounts inbound traffic on EC2 instances (and security groups applied to EC2 instance), which is the requirements for a MountTarget. In our experience usual approach is to create dedicated security group for a MountTarget(s) that will open only NFS port and allow to access a EFS instance from particular list of EC2 instances.
I'd suggest to error out if there is no security group specified instead of trying to guess (and probably guess incorrectly).

Hi @mhrabovcin,

I'd suggest to error out if there is no security group specified instead of trying to guess (and probably guess incorrectly).

Why do you think we're guessing the security group(s)? The names of the security groups to which an instance belongs are generated via the executor by curling http://<IP_ADDRESS>/latest/meta-data/security-groups, which returns a \n delimited list of the names of the security groups to which the requesting instance belongs. That data is then provided to the server via the instance ID's Fields property, which is then used in the VolumeAttach implementation here. The names are used to get the security group IDs, which are then used in the EFS API call.
I'm not sure that I understand where this is guessing. Are you referring to the fact that we're attempting to figure out which SG is intended as the export security group? I think that is probably what you mean here, right? If so, then yeah, I can see that. Perhaps we meet in the middle and try this logic:

If there is a security group named efs or nfs then that is the one assumed to be the default if no security group(s) is/are specified via the configuration file.
If there is not a security group named efs or nfs but there is a security group named default, then it is used if no security group(s) is/are specified via the configuration file.
If there is no security group named efs, nfs, or default and no security group(s) is/are specified in the configuration file on the server then an error is returned from the VolumeAttach operation.

Thoughts?

I think the idea of guessing comes from the fact that when you do get the list of security groups from http://<IP_ADDRESS>/latest/meta-data/security-groups, you still have to pick one to use. Which one?
So yeah, I agree with when you say:

I'm not sure that I understand where this is guessing. Are you referring to the fact that we're attempting to figure out which SG is intended as the export security group? I think that is probably what you mean here, right? If so, then yeah, I can see that.

I'm not sure what the right amount of hand-holding here is. Looking for efs, nfs, default, in decreasing order of precedence, is still guessing. What if they all exist?
Unfortunately, I don't have the real-world experience with EFS to know what is standard/common here. Your heuristic could indeed be a middle ground, but if it is actually more likely to lead to confusion or using the wrong security group, it may indeed be better to require it be in the config file.

How about this? If there is a single security group we use it, otherwise require the Config? Because if there is only one security group, there is no debate which one to use...
--
-a

On Nov 25, 2016, at 4:56 AM, Martin Hrabovcin<EMAIL_ADDRESS>wrote:
Hey @akutz, thanks for explaining the reason for including ec2 API client. If I am reading the code correctly it means that if there is no security group specified in the driver configuration you'll try to guess the security group(s) from a instance.
I don't think this would work in usual way how security groups are structured. There is no reason to open NFS mounts inbound traffic on EC2 instances (and security groups applied to EC2 instance), which is the requirements for a MountTarget. In our experience usual approach is to create dedicated security group for a MountTarget(s) that will open only NFS port and allow to access a EFS instance from particular list of EC2 instances.
I'd suggest to error out if there is no security group specified instead of trying to guess (and probably guess incorrectly).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@akutz: that makes sense to me!

Hi @codenrhoden and @mhrabovcin,
I've updated the code so that the security group names are only used if there is exactly one of them. Additionally, I've introduced efs.disableSessionCache as a boolean config key that can be used to disable the session caching feature.


Add in spark-ec2-git-repo and spark-ec2-git-branch default parameters

New in 1.4.0, they changed the way they did AMI discovery.  See:  https://github.com/apache/spark/commit/b884daa58084d4f42e2318894067565b94e07f9d
Essentially, it relies on a default parameter that we didn’t have.  I followed the examples from the other parameters and added the default in line.
I am spinning up clusters now, it got further than it did before, but they’re not up yet.  If they don’t spin up, I’ll post a comment.  @freeman-lab

After this second commit (adding an option for private-ips) this worked for me with -s 1.  Spinning up 8 now and we'll see.

This looks great, thanks for the fix @andrewgiessel !


Holland version update

https://github.com/rcbops/openstack-ops/blob/e071f104800f249cc1ce6619847258041070b6e1/playbooks/vars/holland.yml#L25
Any reason we do not install holland 1.1.20 (last version with python2 support) for branch ansible-eol, and holland 1.2.1 for master branch?
reference: https://github.com/holland-backup/holland/releases

OK I updated the version