ARROW-4520: [C++] use voidified expr to ignore DCHECK() custom messages in NDEBUG

For release builds DCHECK( x ) << y << z; currently expands to
((void)(x));
while (false) ::arrow::util::ArrowLogBase() << y << z;

This is unreachable which is an error using clang-7

Is it a warning or an error?
I think we should try to follow closely what's in glog
https://github.com/google/glog/blob/master/src/glog/logging.h.in#L1004

Is the PR missing something? AFAICT it's only replacing a while (false) with another while (false)...

@pitrou Sorry, I didn't rename the PR after changing my approach

So unreachability wasn't the issue?

Apparently the warning (error with -Werror) isn't emitted if the unreachable expression is cast to void, which is what's in glog

Isn't it what ARROW_IGNORE_EXPR was supposed to do?

ARROW_IGNORE_EXPR was used to cast the condition/operands of checks to void, but the unreachable expression was ArrowLogBase() << extra_messages... which was not cast to void.

We can't pass that to ARROW_IGNORE_EXPR that because extra_messages are not arguments to the DCHECK macros

We can't pass that to ARROW_IGNORE_EXPR that because extra_messages are not arguments to the DCHECK macros

Another suggestion: could we simply use ARROW_CHECK(true) without the while false?

How about the following:

#define DCHECK(condition)       \
  ARROW_IGNORE_EXPR(condition); \
  ARROW_CHECK(true)

#define DCHECK_EQ(val1, val2) \
  ARROW_IGNORE_EXPR(val1);    \
  ARROW_IGNORE_EXPR(val2);    \
  ARROW_CHECK(true)

// [etc.]

Hopefully that can please all compilers.

I'm fine with it; I was just following @wesm's recommendation to use glog's solution

@pitrou your proposed solution seems OK, might want to run a benchmark that involves DCHECK in debug builds to make sure the compiler is optimizing away the dead code. I was thinking there's no reason to deviate strongly from what is in glog since someone at Google would have noticed if it was wrong by now

No strong opinion. We can go with this and revisit another time if another compiler complains.

That said, it seems the manylinux crash is specific to this PR. It's also one of the rare CI entries that compiles in release mode...
@bkietz can you rebase?

@pitrou rebased and I still get the segfault in release mode.
truncated stack trace:

Decimal128::ToStringNegativeScale out_of_range error in str.substr
Decimal128::ToString
Decimal128Array::FormatValue
DecimalValue.as_py

full stack trace:
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff7805801 in __GI_abort () at abort.c:79
#2  0x00007fffe679d469 in __gnu_cxx::__verbose_terminate_handler () at /home/rdonnelly/mc/conda-bld/compilers_linux-64_1534627447954/work/.build/x86_64-conda_cos6-linux-gnu/src/gcc/libstdc++-v3/libsupc++/vterminate.cc:95
#3  0x00007fffe679bb1b in __cxxabiv1::__terminate (handler=<optimized out>) at /home/rdonnelly/mc/conda-bld/compilers_linux-64_1534627447954/work/.build/x86_64-conda_cos6-linux-gnu/src/gcc/libstdc++-v3/libsupc++/eh_terminate.cc:47
#4  0x00007fffe679bb54 in std::terminate () at /home/rdonnelly/mc/conda-bld/compilers_linux-64_1534627447954/work/.build/x86_64-conda_cos6-linux-gnu/src/gcc/libstdc++-v3/libsupc++/eh_terminate.cc:57
#5  0x00007fffe679bd36 in __cxxabiv1::__cxa_throw (obj=obj@entry=0x555555fe1be0, tinfo=0x7fffe6832e78 <typeinfo for std::out_of_range>, dest=0x7fffe67a76d6 <std::out_of_range::~out_of_range()>)
    at /home/rdonnelly/mc/conda-bld/compilers_linux-64_1534627447954/work/.build/x86_64-conda_cos6-linux-gnu/src/gcc/libstdc++-v3/libsupc++/eh_throw.cc:93
#6  0x00007fffe67b5b49 in std::__throw_out_of_range_fmt (__fmt=<optimized out>) at /home/rdonnelly/mc/conda-bld/compilers_linux-64_1534627447954/work/.build/x86_64-conda_cos6-linux-gnu/src/gcc/libstdc++-v3/src/c++11/functexcept.cc:96
#7  0x00007fffe5926a0b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_check (__pos=2, __s=0x7fffffff6920 "", this=<optimized out>) at /usr/bin/../lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/basic_string.h:302
#8  std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::substr (__pos=2, __n=18446744073709551615, this=<optimized out>) at /usr/bin/../lib/gcc/x86_64-linux-gnu/7.3.0/../../../../include/c++/7.3.0/bits/basic_string.h:2807
#9  arrow::ToStringNegativeScale (adjusted_exponent=<optimized out>, str=..., is_negative=<optimized out>) at ../../src/arrow/util/decimal.cc:107
#10 arrow::Decimal128::ToString[abi:cxx11](int) const (this=<optimized out>, scale=<optimized out>) at ../../src/arrow/util/decimal.cc:127
#11 0x00007fffe585614c in arrow::Decimal128Array::FormatValue[abi:cxx11](long) const (this=0x555555f15900, i=<optimized out>) at ../../src/arrow/array.cc:350
#12 0x00007fffe5f9f926 in __pyx_pw_7pyarrow_3lib_12DecimalValue_1as_py(_object*, _object*) () from /home/bkietz/arrow/python/pyarrow/lib.cpython-36m-x86_64-linux-gnu.so

@bkietz Do you want me to take a look?

@pitrou That would be appreciated, thanks. I'll try tweaking logging.h too

@bkietz Looks like my intuition was right. I fixed the issue here with the following diff:
diff --git a/cpp/src/arrow/util/logging.h b/cpp/src/arrow/util/logging.h
index dfdfc71d..38ada2fd 100644
--- a/cpp/src/arrow/util/logging.h
+++ b/cpp/src/arrow/util/logging.h
@@ -80,22 +80,24 @@ enum class ArrowLogLevel : int {
 #ifdef NDEBUG
 #define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING

-#define DCHECK(condition) \
-  while (false) ARROW_CHECK(condition)
-#define DCHECK_OK(status) \
-  while (false) ARROW_CHECK_OK(status)
+#define DCHECK(condition)       \
+  ARROW_IGNORE_EXPR(condition); \
+  ARROW_CHECK(true)
+
+#define DCHECK_OK(status)    \
+  ARROW_IGNORE_EXPR(status); \
+  ARROW_CHECK(true)
+
 #define DCHECK_EQ(val1, val2) \
-  while (false) ARROW_CHECK((val1) == (val2))
-#define DCHECK_NE(val1, val2) \
-  while (false) ARROW_CHECK((val1) != (val2))
-#define DCHECK_LE(val1, val2) \
-  while (false) ARROW_CHECK((val1) <= (val2))
-#define DCHECK_LT(val1, val2) \
-  while (false) ARROW_CHECK((val1) < (val2))
-#define DCHECK_GE(val1, val2) \
-  while (false) ARROW_CHECK((val1) >= (val2))
-#define DCHECK_GT(val1, val2) \
-  while (false) ARROW_CHECK((val1) > (val2))
+  ARROW_IGNORE_EXPR(val1);    \
+  ARROW_IGNORE_EXPR(val2);    \
+  ARROW_CHECK(true)
+
+#define DCHECK_NE DCHECK_EQ
+#define DCHECK_LE DCHECK_EQ
+#define DCHECK_LT DCHECK_EQ
+#define DCHECK_GE DCHECK_EQ
+#define DCHECK_GT DCHECK_EQ

 #else
 #define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_FATAL

DCHECK_OK(s) provided the guarantee that s will be evaluated, so the test failure was caused by some places where it is being used as a synonym for ABORT_NOT_OK

@bkietz these cases should not abort in release builds. I think what can be done in such cases is this:
Status s = $STATEMENT;
DCHECK_OK(s);

I do think they should either bubble an error, or abort. If we are not bubbling an error, it's usually because the function doesn't return an error code, so we have to abort. Ignoring errors is extremely bad.

Also I have no opinion on whether expressions in debug checks should be always evaluated ornot.

I do think they should either bubble an error, or abort.

Well, I think the idea here is that these particular function calls "can't fail" unless something is implemented wrong by an Arrow developer. It might be a good idea to refactor the code that is being used into two variants:

A "can't fail" version that does not return Status
A "can fail" version that returns Status, for example to guard against bad user input

The way that DCHECK is used in practice, it is expected that the statements inside have no cost in release builds. I think that DCHECK_OK might have been a special case

We could use ARROW_LOG(DFATAL), which is FATAL in debug and WARNING in release

If it's a fatal condition (for example a null pointer where you would have expected actual data), then I don't think it makes sense to just warn and carry on.

Here's the code
  DCHECK_OK(Divide(kTenTo36, &top, &remainder));

Divide has failure modes that simply do not apply here at all

Divide by zero
BuildFromArray overflowing

If either of these cases occurs, there is a flaw in the implementation that can be caught by DCHECK in debug builds. It should not be possible to trigger these error conditions in release builds if the tests pass in debug builds

Right, so either we don't check at all, or we abort on error (since it's a critical error). I don't think warning makes sense at all.

Yes, I think the correct option is:

Don't check in release builds: execute the statement, but ignore the Status code
DCHECK(s.ok()) in debug builds

So the implementation of DCHECK_OK should be
do {
  Status _s = STATEMENT;
  DCHECK(s.ok());
} while (0)

If STATEMENT has side effects then it will be executed in both debug and release builds

@wesm @pitrou how does that look?

also: what happened with appveyor? It shows green here on the PR (for me at least) but MinGW seems to have failed

@bkietz The MinGW 32-bit failures are expected for now, that's why they don't fail the build ;-)

I find this solution confusing. If DCHECK_OK always evaluates its argument, I would expect other DCHECK macros to do as well. Otherwise it's too error-prone.
I'm not strongly attached to one or the other alternative (either we check in release mode or not), but it should be consistent accross the board.

But, really, since this PR started from the desire to fix a compilation failure, I think the simplest thing is to fix the compilation failure without changing the current semantics. We can revise the semantics in another JIRA / PR but that sounds a bit low-priority to me.

https://issues.apache.org/jira/browse/ARROW-4664

This seems spurious: https://travis-ci.org/apache/arrow/jobs/497095148#L3065

rebased

@wesm the CI failure is unrelated to this PR, see ARROW-4684


GPFS compression information for Gaea system

We need to add information on Gaea's GPFS compression.  ATT, compression of F6 is disabled.
The following is the policy for compression on F5:
/* Macros */
define(excluded_files,(PATH_NAME LIKE '%/.SpaceMan%' OR
PATH_NAME LIKE '%/fs_audit_log/%' OR
PATH_NAME LIKE '%/.snapshots/%' OR
PATH_NAME LIKE '%/.msgq/%'))

/* I.E. Files must be idle for 12 hours before being a candidate for compression */
define(access_buffer_time_passed, ((CURRENT_TIMESTAMP - MODIFICATION_TIME) > (INTERVAL '168' HOURS) AND (CURRENT_TIMESTAMP - ACCESS_TIME) > (INTERVAL '168' HOURS)))
/* Ensure compression on qualified files */
RULE 'compress-large-files-on-hdd' MIGRATE COMPRESS('lz4') FROM POOL 'capacity' WHERE not(excluded_files) AND (KB_ALLOCATED >= 4096) AND access_buffer_time_passed

The following should be included as well (but not as question/answer or a FAQ):

Can users decompress their files?
YES, if they can run the following it’ll decompress immediately: mmchattr --compression no -I yes <file>
Are files written uncompressed to disk?
YES. They are compressed upon the execution of our compression cronjob or the above mmchattr
Are they decompressed on read?
NO. Reading a file does not change its status on disk.
If compressed, are they decompressed on write?
KIND OF. Writes only decompress and modify the relevant portion of the file. Upon doing so, they are left “illcompressed” and will be re-compressed later upon execution of our cron job or an mmchattr to compress
Is there some “mmlscompressratio [file | directory]” type command?
NO. Users will need to use commands that return the full size of files (ls) and divide that by something that shows the on disk space usage of a file (du)
How can we/users know how much space will be used after files are decompressed?
Per file: ls will show you the decompressed size of the file, du will show you the on-disk size.
In total: that would probably require a very invasive script doing a lot of ls’s and du’s. Chris Coffman said F2 saw about 1.35:1 the times he checked; from what I’ve seen on F5 from light poking, it’s going to vary wildly.

Misc
IBM recommends compression for cold files. IDK what the workflows are like, but it’s possible that we could modify the ILM compress policy to wait longer on compression (currently 168 hours/7 days)

Added as part of #296


Bumps base images

This PR bumps base images used as a base for all cert-manager component images.
Versions were bumped by running ./hack/latest-base-images.sh
Bumps base images

/kind cleanup
Signed-off-by: irbekrm<EMAIL_ADDRESS>

Part of #4033

I think this PR is missing the second step from #4033  -

If ./hack/latest-base-images.sh produces any differences, copy the digests for the amd64 images into images.bzl

Thanks @SgtCoDFish !

/test pull-cert-manager-make-test

/lgtm
/approve


code.tizen.org - example - Index Scrollbar not working

Open
https://code.tizen.org/demos?path=1.0%2Fexamples%2Fmobile%2FUIComponents%2Fcomponents%2Fnavigationelements%2Findexscrollbar.html
Click on letters, numbers on the right side
Observe: no scrolling occurs

Repaired by #147

works well


Allow threads to timeout in the thread pool

As you asked, I did not go far in the config wiring. But if this solution is ok, I can do it if you want.
Currently, I only verified on a real project that the sending thread expired correctly after 1 second. But to check that threads are correctly expiring, it is possible to write a unit test. I can do it, but I prefer first to validate this!
About the exception bubbling up in the ThreadPoolExecutor and not being logged through SLF4J should I create a separate issue?

No actually the error should be logged in 5.1.6. In 6.0.0 (develop) it is allowed to bubble up as it is now. I'll reapply that change. Or you can do it, no problem.

Currently, I only verified on a real project that the sending thread expired correctly after 1 second
So I'm seeing 1 preconfigured in the OperationalConfig and MILLISECONDS on the executor. I suppose you tested it with 1000 instead of 1?

No actually the error should be logged in 5.1.6. In 6.0.0 (develop) it is allowed to bubble up as it is now. I'll reapply that change. Or you can do it, no problem.
Ok thank you, I will reapply the change!

you tested it with 1000 instead of 1?
My mistake... I tested with 1 millisecond. Actually I first used milliseconds, but for that it would have been better to use a Long duration instead of an Integer one. And to use a Long, more work would have been necessary in the configuration part, since no method currently exists to parse a Long value.
Anyway, I can make this pull request better (unit testing for timeout, correct seconds or milliseconds timeout, configuration wiring), but before I spend too much time on this, I would prefer to be sure you are ok with what I will be doing.
Do these changes seem acceptable for you or do you think the value is too low for the added complexity?

I had some difficulty deciding whether we're solving any problem here. I still don't think so. However, using a custom ThreadPoolExecutor, users are able to fine tune the threading policy of Simple Java Mail.
I propose making both core and max threads configurable as well as the timeout.
I'm still working on understanding why we need core threads at all when we're allowing them to time-out anyway as not to block the JVM from shutting down. Have an SO question out to help me with that (more questions in the comments there): https://stackoverflow.com/questions/55879100/with-threadpoolexecutor-what-is-the-difference-between-allowcorethreadtimeout-a


Feature/25 setup socketio

#25

To see the specific tasks where the Asana app for GitHub is being used, see below:

https://app.asana.com/0/0/1207662929082466
https://app.asana.com/0/0/1207380167122126

The faker seeder seems invalid on seeding fake phone number @wayangalihpratama

I solved the socketio testing problem by using AsyncMock on test case.

Hi @kjkoster ,
I have tried to implement the queue message format on both the frontend and backend. However, some values are still static and will be updated later once we connect all the elements (Slack, Twilio, etc.).


Fix podman build completions

Fixes: https://github.com/projectatomic/libpod/issues/1076
Signed-off-by: Daniel J Walsh<EMAIL_ADDRESS>

@vrothberg @umohnani8 @baude @TomSweeneyRedHat PTAL

LGTM

Tested locally.
LGTM

@rh-atomic-bot r+


Documentation rendering problem

The type of l10n.bundle has an anonymous object type part:
 export const bundle: { [key: string]: string } | undefined;

In the documentation for the l10n namespace, that part is not rendered:

Hi @md2perpe Thank you for noticing and reporting this rendering issue. API generation is handled by code in our private website repository so moving this issue there. As you point out, vscode.d.ts (picked up by editor IntelliSense) is the source of truth so extension authors shouldn't be blocked until this rendering issue is fixed.


Id is not added in the info object.

id is a required property of info which is not present in the converted schema.
Soln: prat0318/swagger-converter@47737e3cc2f7457e11c3af3e5bed591199da17a0

I think you meant https://github.com/prat0318/swagger-converter/commit/2a4e2006bb442c81c31ee4f2847b559a235a3432
That looks good. Could you make a PR with that change?

@mohsen1 Both patches:
https://github.com/prat0318/swagger-converter/commit/47737e3cc2f7457e11c3af3e5bed591199da17a0
and
https://github.com/prat0318/swagger-converter/commit/2a4e2006bb442c81c31ee4f2847b559a235a3432
made their way into swagger-converter so you can close this issue.

👍


NPM Bundle is over 1.6Mb

I'm submitting a ...
[ ] bug report
[ ] feature request
[x] question about the decisions made in the repository
[ ] question about how to use this project

Summary
Hi there 👋
I've been using this npm package on a custom Solana explorer I'm building and I've noticed how heavy this package is — 1.6Mb.
This is due to the fact that the package imports the entire JSON file as a fallback mechanism should the API request fail to work.
https://github.com/solana-labs/token-list/blob/21263a9bc5f2dc0dbde978c496ad0165a9c1f760/src/lib/tokenlist.ts#L98-L101
Whilst this is a nice thought, it makes this package almost unusable because 1.6Mb is gigantic for an npm package and at this point, since the entire JSON payload is loaded on the client, you might as well use that rather than making an extra API call.
I ended up copy/pasting the code inside the tokenlist.ts file into my app without importing the tokenlist.json file and my bundle went back to a normal size. Let me know if you'd like me to submit a PR for this. Otherwise, I'm happy to continue using my custom solution.
Cheers. 🍺

thanks loris, love your work on tutorials!  could you please submit a PR for this?

Thanks! Of course, I've been experimenting with it this morning.
Sadly, I can't just remove the whole Static strategy from the main module as I did locally because that will fail the tests.
However, I've found a way to make this library tree-shakeable by making the user explicitly import the strategy they want so all other strategies can be pruned at compile time. I'll submit a PR soon. :)

Hey @keone 👋
Just pinging you in case the PR (#12329) got lost in the lot but absolutely zero rush on my side! ☺

The bundle size are now 3.6 MB minified.

Is there any workaround for this issue?


update jacoco version to 0.8.9

Why are the changes needed?
Just a test to fix EOF problem in github ci
Brief change log

Upgrade jacoco version to 0.8.9

It doen't matter with the version of jacoco


Updated SvelteGenericCrudTableService.js

Made the changes in update / delete functions. Instead of 'getTable', used 'getRow' method to pull the 'parentRow' information.

ty for contribution.


boto3.s3.transfer.create_transfer_manager() type annotations do not match implementation

Implementation accepts boto3.s3.transfer.TransferConfig: https://github.com/boto/boto3/blob/develop/boto3/s3/transfer.py#L147
But type annotations require botocore.config.Config:
from botocore.config import Config

def create_transfer_manager(
    client: BaseClient,
    config: Config,
    osutil: Optional[OSUtils] = ...,  # type: ignore
) -> TransferManager: ...

Version: boto3/boto3-stubs 1.24.31

Thank you for the report! Fixed, will be released ASAP.

Fixed. A new version of boto3-stubs should be released automatically next Monday.

Please let me know if boto3-stubs 1.24.32 works for you as expected.


test_gateway_executes_swaps_between_connected_federations fails often

See e.g. https://github.com/fedimint/fedimint/actions/runs/6943815403/job/18889679969#step:6:2342 and other builds from the merge queue in #3666.

I just had a failure again, so I looked into the test and wondered if this && shouldn't be a ||:
https://github.com/fedimint/fedimint/blob/4c6766a2386ba0890d875fa7c76dd1b2645ccd2f/gateway/ln-gateway/tests/integration_tests.rs#L1195-L1197
Is it really enough to wait for any of the two values to change? E.g. if the second one changed, we exit the retry loop because of that (the first value is unchanged) and then check in the assertion that the first value is changed in a specific way, that sounds like it could fail.

I included my suspected fix in https://github.com/fedimint/fedimint/pull/3703, seems to work but unclear if that was just luck.

I just had a failure again, so I looked into the test and wondered if this && shouldn't be a ||:
https://github.com/fedimint/fedimint/blob/4c6766a2386ba0890d875fa7c76dd1b2645ccd2f/gateway/ln-gateway/tests/integration_tests.rs#L1195-L1197
Is it really enough to wait for any of the two values to change? E.g. if the second one changed, we exit the retry loop because of that (the first value is unchanged) and then check in the assertion that the first value is changed in a specific way, that sounds like it could fail.

oh my boolean! 🤦🏿♂

Haven't seen it happen since my fix got merged, closing. Feel free to reopen if it happens again.


Making a phone call

I as a gssoc'22 contributor would like to add this feature. Could you please assign this to me

Hi
I am a GGSOC Contributor 2022
Please assign me this issue


Opinions on functions vs transforms?

Great work on this library. I was wondering about your perspective on functions vs transforms
The readme suggests:
"While Transforms are the preferred way to change one value into another value, Jexl also allows top-level expression functions to be defined"
And I see that functions are actually a recent addition, so this is pretty cool, but do you have any reasons why transforms would be preferred?

Hi Colin!
At the end of the day it all boils down to personal preference, as there's no functional advantage to one over the other. The readme suggests transforms for two main reasons:

Jexl was written for its expressions to be understood and written by savvy non-programmers. If you're not already used to the function(arg) syntax employed by C-like languages, then the subject-verb left-to-right flow of price|roundUp can feel more natural than roundUp(price). I pulled that syntax from the family of popular template languages based on Django's (Twig, Nunjucks, etc) where those expressions were also optimized for folks who aren't traditionally programmers.
With the introduction of functions that overlap the functionality of transforms, I felt it was good form to provide a guideline for the community so that Jexl code could be shared without having to be concerned with a codebase's internal conventions. So the options there were 1. Recommend using functions and deprecate transforms because they no longer have a use, or 2. Recommend using transforms for their original purpose and reap the ease-of-use benefits of my first point above. So I decided on #2.

So there you have it! If the function syntax makes more sense to you and anyone you have that might be writing these expressions, by all means you should use that :). If the above reasons could be impactful for you, though, transforms still have a nice place within the language definition.
 :)


update tags, add my 3rd boot, and mvp link

:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

rd/huan-li.yml
:white_check_mark:Succeeded

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

Your change is live now. Please review it from: https://rd.microsoft.com/en-us/huan-li


make ciliumidentities scalable

ciliumidentities won't scale well due the fact that all cilium-agents are watching for this resource and all nodes update its status as soon they are using the identity.

Is it primarily just status? This seems like a recurring theme which it'd be nice to come up with a general approach for how we address it.
Offloading to kvstore is an approach we've used before, CNPNodeStatus in particular:
http://docs.cilium.io/en/latest/kvstore/#ciliumnetworkpolicynodestatus

That's also a good plan as well.

@aanm This issue was resolved in v1.8.0 wasn't it?

Yes, Cilium identities no longer have the nodes' status. Closing...

Hi Aanm/Joe, thanks for updating this issue.
was this fix backported to 1.6.x and/or 1.7.x branch?
we are currently on 1.6.x and use etcd as kv store (with plans to move to 1.7.x and then to 1.8.x in next quarter).
I am wondering if we should wait for upgrades to 1.8.x before starting to use cilium identities CRD.
someone from our team mentioned that the cilium can work with both cilium-identity CRD (for allocation) and existing kv store (for scaling).
any pointers (may be some documentation or link to PR that fixes this issue) to help make a decision (to enable CRD on 1.6.x or not), will be very helpful.
many Thanks
Rajat Jindal


Remove Javascript for New Relic

Nobody seems to be using whatever data is gathered by this behemoth. When unminified, it is over 200 lines of code, processed on every page load. 👎

Def not used by the data team. +1

Thanks, @mstanleyjones :)


feat(dynamo): Send/Receive Nodes for Dynamo Player

Description
As mentioned by Nick_Boyts from community.
Users cannot interact with Speckle via Dynamo Player. Currently, it is necessary to open Dynamo and click the blue button on the Send/Receive node to receive/send data from Speckle.
Expected Behaviour
Dynamo Player-friendly Send and Receive nodes. Nodes that receives/sends when users run Dynamo definition.
Related Community Discussion
https://speckle.community/t/speckle-dynamo-issues-with-player/3463

Fixed in #2040


Viewer + orthanc + docker compose = CORS

Hello,
I am trying to setup a basic infra based only on docker images. I took inspiration from this repo so far https://github.com/mjstealey/ohif-orthanc-dimse-docker/ it was working as expected. I am now in the process of upgrading most of the images and I am stuck with a CORS error.
This problem emerged after I did the upgrade from the viewer v1 to viewer v3.
I did my best to find a solution for 2 days now, getting help from other examples, but I can't understand what's wrong with my nginx configuration.
Code is ready to be tested here https://github.com/trypag/ohif-orthanc-postgres-docker
Hopefully it can serve others once it's fully upgraded. I would really appreciate someone's experience on this, thanks :smile:

Hmm... Your config has you proxying requests to orthanc. That means requests should hit localhost:8080/orthanc, not orthanc:8042
Your docker-compose stands up orthanc, but makes sure it's not accessible outside of the container's internal network (this is good and as intended) :+1:
It gives the orthanc container, a name of "orthanc", which is how nginx is able to resolve its api address when proxy passing
I'm pretty sure these fields are your issue: https://github.com/trypag/ohif-orthanc-postgres-docker/blob/master/config/viewer.js#L9-L11
servers: {
    dicomWeb: [
      {
        name: 'Orthanc',
        wadoUriRoot: 'http://orthanc:8042/wado',
        qidoRoot: 'http://orthanc:8042/dicom-web',
        wadoRoot: 'http://orthanc:8042/dicom-web',
        qidoSupportsIncludeField: false,
        imageRendering: 'wadors',
        thumbnailRendering: 'wadors',
      },
    ],
  },

Should probably be:
servers: {
    dicomWeb: [
      {
        name: 'Orthanc',
        wadoUriRoot: 'http://localhost:8080/orthanc/wado',
        qidoRoot: 'http://localhost:8080/orthanc/dicom-web',
        wadoRoot: 'http://localhost:8080/orthanc/dicom-web',
        qidoSupportsIncludeField: false,
        imageRendering: 'wadors',
        thumbnailRendering: 'wadors',
      },
    ],
  },

Thanks, I finally understand how to configure the viewer servers, I also understand why requesting orthanc:8042 was not possible. :+1:
So I made a few modifications (basically used your implementation), I managed to remove the extra nginx and use only the one from the viewer's image to proxy.
I still have CORS issue, but now I think I am just missing the correct headers in nginx.
Code is updated https://github.com/trypag/ohif-orthanc-postgres-docker

I think I should use the same as here right ?
Should I just replace the proxy related code by the root and index ?

I think the viewer's servers are now correctly defined, I am just missing the correct definitions of the headers.
I tried to define those :
location / {
        if ($request_method = 'OPTIONS') {
            add_header 'Access-Control-Allow-Origin' '*';
            add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
            #
            # Custom headers and headers various browsers *should* be OK with but aren't
            #
            add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range';
            #
            # Tell client that this pre-flight info is valid for 20 days
            #
            add_header 'Access-Control-Allow-Headers' 'Authorization';
            add_header 'Access-Control-Allow-Credentials' true;
            add_header 'Access-Control-Max-Age' 1728000;
            add_header 'Content-Length' 0;
            return 204;
        }
        if ($request_method = 'POST') {
            add_header 'Access-Control-Allow-Origin' '*';
            add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
            add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range';
            add_header 'Access-Control-Expose-Headers' 'Content-Length,Content-Range';
        }
        if ($request_method = 'GET') {
            add_header 'Access-Control-Allow-Origin' '*';
            add_header 'Access-Control-Allow-Methods' 'GET, POST, OPTIONS';
            add_header 'Access-Control-Allow-Headers' 'DNT,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Range';
            add_header 'Access-Control-Expose-Headers' 'Content-Length,Content-Range';
            add_header 'Access-Control-Allow-Headers' 'Authorization';
            add_header 'Access-Control-Allow-Credentials' true;
        }

        root   /usr/share/nginx/html;
        index  index.html index.htm;
        try_files $uri $uri/ /index.html;
        add_header Cache-Control "no-store, no-cache, must-revalidate";

        # proxy_http_version 1.1;

        # proxy_set_header Host               $host;
        # proxy_set_header X-Real-IP          $remote_addr;
        # proxy_set_header X-Forwarded-For    $proxy_add_x_forwarded_for;
        # proxy_set_header X-Forwarded-Proto  $scheme;

        # proxy_pass http://orthanc:8042;
    }

Clearly, it's still a failure ^^

It's working ! I had my viewer's config wrong again (orthanc instead of localhost, which made no sense).
I am still interested in how to setup CORS though :)

I can define a simple use case to illustrate the workings :

viewer is hosted locally at :80
orthanc is hosted on a remote server at x.x.x.x:8042

My current thinking is that nginx (hosted locally at :80) should proxy requests to x.x.x.x:8042 and allow cross domain in the location \ section right ?
Though I can't understand how the viewer's content can be delivered :
location / {
          alias  /var/www/html/;
          index index.html;
          try_files $uri $uri/ /index.html;
          add_header Cache-Control "no-store, no-cache, must-revalidate";
        }

AND at the same time proxied like in here
Not sure if I am clear ?

You generally don't want to expose orthanc to the open web. See: https://book.orthanc-server.com/faq/security.html

If Orthanc is put on a server that can be contacted from Internet, put Orthanc behind a reverse proxy, and let this reverse proxy take care of the HTTPS encryption.

This is our nginx "proxy pass" configuration. nginx is handling the web requests, https, etc.
If you have orthanc behind a web server like nginx on a different domain/origin/port, then you can explicitly set CORS headers in the responses nginx returns using the nginx config:
https://enable-cors.org/server_nginx.html

Ok I think I got it :smile:
Still not perfect without a proper use case to implement, but I can go further now.
Thank you so much for your help, it was clear and really helpful.
Sorry for all the incoming issues I will be opening during the following weeks, as I try to get a deeper understanding of the viewer. Thanks @dannyrb  :hand:

@trypag where are you at with this?  Do you feel comfortable that I could recreate your docker setup and deploy my own version?  It seems that what you have come up with is exactly what I'm trying to do.  It would be nice to not have to recreate the wheel.  I'm hoping for a local instance of OHIF and Orthanc deployed on one machine but I know just enough to make my life miserable about this whole process.  I'm trying to help out a friend who needs a reliable way to host and recall US images.  We like Orthanc but it's not as user friendly as I want.

Hello @supermansrf , I managed to make a proper docker config, everything is up at https://github.com/trypag/ohif-orthanc-postgres-docker
You should be able to run it easily on your side. Let me know if there is any issue.

Thanks for the quick reply. I’ll start messing with it.

hey @dannyrb can you have a look at our discussion over here:
https://github.com/trypag/ohif-orthanc-postgres-docker/issues/1
And see if you can shed any light on this issue?

Not sure if any of this is any help but...


lb_test gives seg fault in multicore build

Original issue: https://charm.cs.illinois.edu/redmine/issues/769

In multicore-linux64 build, doing a make test on tests/charm++/load_balancing/lb_test gives seg fault at the exit.
Stack trace:
TIME PER STEP	80	1.168167	0.006833
TIME PER STEP	90	1.216015	0.047848
All done
[Partition 0][Node 0] End of program

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff6fec700 (LWP 26316)]
__run_exit_handlers (status=0, listp=0x7ffff73a4688, run_list_atexit=true) at exit.c:50
50	exit.c: No such file or directory.
(gdb) bt
#0  __run_exit_handlers (status=0, listp=0x7ffff73a4688, run_list_atexit=true) at exit.c:50
#1  0x00007ffff7028635 in __GI_exit (status=<optimized out>) at exit.c:100
#2  0x00000000006abf86 in LrtsExit () at machine.c:1826
#3  0x00000000006a727c in ConverseExit () at machine-common-core.c:1352
#4  0x0000000000593f21 in _exitHandler (env=0xb58920) at init.C:600
#5  0x00000000006ae3c4 in CmiHandleMessage (msg=0xb58920) at convcore.c:1633
#6  0x00000000006ae759 in CsdScheduleForever () at convcore.c:1861
#7  0x00000000006ae69c in CsdScheduler (maxmsgs=-1) at convcore.c:1797
#8  0x00000000006a710d in ConverseRunPE (everReturn=0) at machine-common-core.c:1246
#9  0x00000000006a4827 in call_startfn (vindex=0x1) at machine-smp.c:414
#10 0x00007ffff7bc4e9a in start_thread (arg=0x7ffff6fec700) at pthread_create.c:308
#11 0x00007ffff70e038d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#12 0x0000000000000000 in ?? ()

Original author: Nikhil Jain
Original date: 2015-08-18 15:21:48

Hopefully, it should be working now. Please test and confirm.

Original date: 2015-08-18 16:20:20

It still fails for me with the latest version of charm, but It only fails once in 4 runs or so. Works most of the time. I'm testing on valor.

Original date: 2015-08-18 22:08:32

It fails for me on the latest version of Charm on about 1 run out of 8, but when it does so, it usually segfaults during startup.  Clearly this is not the same bug.
CharmLB> Load balancer assumes all CPUs are same.
make: *** [test] Segmentation fault (core dumped)


Implement ArtPoll request/response handling

The ArtNet protocol mandates that every node replies to ArtPoll packets for discovery of nodes in the network. We don't currently do this, but in order to be compliant with the protocol, we need to implement this.

Currently implemented upstream in https://github.com/jeffreykog/node-artnet-protocol


add option to FASTA reader to ignore blank/whitespace-only lines

Would be useful to add an option to the FASTA reader to ignore blank/whitespace-only lines.

This change should happen with the FASTQ parser too.

Some unit tests that definitely should exist for this functionality:

The case where an empty line occurs in the middle of a record should fail
The case where an empty line occurs instead of the sequence should fail
The case where an empty line occurs between records should succeed

@anderspitman Would you be up for tackling this issue? Should be relatively straight-forward and will introduce you to IO. I am happy to explain anything in regards to IO as well.

Just curious, is there a standard for these files? Does it say anything
about blank lines?
On Mar 6, 2015 1:08 PM, "Evan Bolyen"<EMAIL_ADDRESS>wrote:

Some unit tests that definitely should exist for this functionality:

The case where an empty line occurs in the middle of a record should
fail
The case where an empty line occurs instead of the sequence should
fail
The case where an empty line occurs between records should succeed

—
Reply to this email directly or view it on GitHub
https://github.com/biocore/scikit-bio/issues/781#issuecomment-77626587.

I haven't checked in a while but can we also add that it should report
what was the last record it successfully parsed? Usually useful to
figure out what's wrong with your file.
On (Mar-06-15|12:08), Evan Bolyen wrote:

Some unit tests that definitely should exist for this functionality:

The case where an empty line occurs in the middle of a record should fail
The case where an empty line occurs instead of the sequence should fail
The case where an empty line occurs between records should succeed

Reply to this email directly or view it on GitHub:
https://github.com/biocore/scikit-bio/issues/781#issuecomment-77626587

From the file format descriptions I could find for FASTA/Q, blank/whitespace-only lines aren't part of the format. However, we've had users with files that had a trailing blank line at the end of the file, or with records separated by newlines (maybe for readability?).
I think we should support ignoring these lines only if they appear between records. If they appear in the middle of a record it's hard to tell if a user incorrectly catted a file together, if there was an empty sequence (not allowed), etc.
I'm on the fence about whether this behavior should be the default or not. Thoughts?

That's very reasonable.
I don't have any thoughts regarding whether it should be possible to error
or not on blank lines.
2015-03-06 13:30 GMT-07:00 Jai Ram Rideout<EMAIL_ADDRESS>
From the file format descriptions I could find for FASTA/Q,
blank/whitespace-only lines aren't part of the format. However, we've had
users with files that had a trailing blank line at the end of the file, or
with records separated by newlines (maybe for readability?).
I think we should support ignoring these lines only if they appear
between records. If they appear in the middle of a record it's hard to
tell if a user incorrectly catted a file together, if there was an empty
sequence (not allowed), etc.
I'm on the fence about whether this behavior should be the default or not.
Thoughts?
—
Reply to this email directly or view it on GitHub
https://github.com/biocore/scikit-bio/issues/781#issuecomment-77630005.

@ebolyen Somehow I didn't notice you tagged me until @jairideout suggested this issue today. Sorry about that. Happy to work on this.

@jairideout we want to implement this as an option and not default behavior, correct? The current documentation explicitly says blank spaces aren't valid.
@ebolyen Looking through the potential test cases you mention. I know in the interest of performance we don't currently validate records unless/until the user calls is_valid, correct? In that case do we want to do whitespace validation every time the blank space feature is enabled, or roll whitespace validation into is_valid? I'm assuming the latter.

we want to implement this as an option and not default behavior, correct?

@ebolyen and I think this should be the default (and only) behavior, no flag/option necessary.

In that case do we want to do whitespace validation every time the blank space feature is enabled, or roll whitespace validation into is_valid?

I think this whitespace handling should be enabled at all times (it already is, so we shouldn't have too much of a performance hit). BiologicalSequence.is_valid checks whether the characters are within the IUPAC alphabet so we leave that check off since it could have a large performance impact.

Ah ok I didn't realize we were already doing some validation outside of is_valid.

I just came across a file, downloaded from NCBI, which had an empty line at the end. I haven't tested @anderspitman PR yet, hope it fixes that :)

@Jorge-C it should work. I'm happy to try it real quick if you want to link or send me the file.

Thanks!! It's a small genome, can be downloaded here. If the link expires, it can also be found here. Then click on "send" and choose destination "file".

Thanks! It always helps to test with real data early on. I an confirm that that sequence does not work on master, but does work after the changes in this PR, so that's a good sign.

Sweet, thanks for checking!

@jairideout @ebolyen concerning FASTQ files. It appears we're using some of the same test files as the Open Bioinformatics Foundation. One of the files, error_trunc_at_seq.fastq, is considered invalid by OBF but would be valid after implementing the functionality in this issue. Should we diverge here or not allow blanks in FASTQ? I'm assuming diverge but want to make sure.

Sorry I meant error_no_qual.fastq, and I believe the file is still invalid but it shouldn't fail because of the blanks.

Wow, wrong again. That file is completely missing quality scores (who would have guessed). Ignore the last 3 comments.

Here's a more useful question: Should we give preference to blank line exceptions or invalid qual character exceptions with qual data?

@jairideout FYI I rediscovered what that last question was about. Consider the following FASTQ file:
@foo bar baz
AACCGG
+
MNO

@QR
@bar baz foo
TTGGCC
+
876543

In this case the error is somewhat ambiguous, due to the fact that '@' is a valid quality character. Is there a blank space embedded within the quality scores of the first sequence, or is the second sequence (id 'QR') missing it's data, in addition to the first sequence having truncated quality scores? You can get a reasonable answer by counting the quality scores. If the number of quality scores is correct but there's a blank line embedded, you know what's wrong. We're not going that far currently. I think it's reasonable to just assume a blank line error in this case, which is what I'm doing currently in #893.

This is an interesting case, thanks for brining this up. I agree that a blank line error makes sense, since a blank line occurs before we've found the expected number of quality scores. Thus, we know that a blank line occurred within a record, which we aren't allowing. @ebolyen do you agree with this behavior?

@jairideout That makes sense to me.

I haven't checked in a while but can we also add that it should report
what was the last record it successfully parsed? Usually useful to
figure out what's wrong with your file.

@ElDeveloper can you please create a new issue for this feature? I agree this would be good to have but it didn't make it into #893 / #916.


Laik/61 ssl

Resolves #61
DCO1.1 Signed-off-by: Narine Cholakyan<EMAIL_ADDRESS>

Hi narinearraul,
Thanks for submitting this pull request!
I can confirm that the DCO1.1 sign-off has been included. It is okay to process this pull request.
dco-bot


How to use Drupal Bootstrap Theme with Sass on Drupal VM?

On Windows 10 I have successfully installed Drupal VM (Vagrant, VirtualBox).
I want to start theming and installed:

The Drupal Bootstrap Theme: www.drupal.org/project/bootstrap
The Drupal Bootstrap Library Module: www.drupal.org/project/bootstrap_library
Bootstrap from getbootstrap.com
The CompassApp locally installed on Windows 10, compass.kkbox.com
A subtheme of the Drupal Bootstrap Theme, generated and active

How can I start using and modifying this Drupal subtheme "Bootrap subtheme" using SASS, compiling the SASS files (maybe using the Compass App)?
What is the best way to start and make my own theme changes using SASS? Thank you.

@webprogrammierer the code base should be available to your host as defined in vagrant_synced_folders you can use any development tools directly on the host machine.
This is no different to using a local stack like ADD or WAMP.

Thank you. The vagrant_synced_folders  is /var/www/drupalvm like default.
There is no SASS installed. How can I install SASS on Drupal VM? How can I install SASS using Vagrant?
Are there many ways to compile Sass files on Drupal VM? Which?

@webprogrammierer I'd recommend doing this on the host machine (Windows) rather than inside the VM.
The codebase will be available inside windows, how have you done it in the past?

Okay, I can understand.
Why is it better to do this on the host machine (Windows)?
I am new to Drupal VM and I am new to Bootstrap Theme with SASS.
Maybe there is a good tutorial starting with Bootstrap Theme using SASS on a dev environment with a virtual machine (Ubuntu, Drupal VM).

Drupal VM should support any theme so there's nothing specifically required to use the Bootstrap Theme and/or SASS, maybe look for a Windows tutorial for setting up SASS.

Looking for tutorials for setting up SASS you can always find solutions to setup Sass on the linux based server (in our case inside the VM) rather than on Windows.
So I am still not sure what the better way is to do.

Maybe someone else can help, it's been almost 10 years since I last used Windows for dev.

Yes, it would be great, if somebody else can help to get different solutions.

@webprogrammierer - I know @sarahjean has done something like this in the past (installing the tooling inside the VM), but I generally do all my dev work/tooling on the host, and just use the VM to run the site/display the artifacts built by SASS/other tools installed on my host, so I can't be too much help here.
I'll leave this ticket open a little longer, but this is a more general question that might be better to ask in a place like Drupal Answers.

In the meantime I successfully installed SASS and Compass within the Drupal VM:
I installed SASS like this (or similar): http://sass-lang.com/install
I installt Compass like this: http://thesassway.com/beginner/getting-started-with-sass-and-compass
Both, SASS and Compass work fine within the Drupal VM (using Vagrant SSH).
The only thing is, that you have to use the --poll option if you are watching files or directories:
sass --watch --poll scss:styles
or
compass watch --poll
Whithout the --poll option changes are not recognized.
Great - it works.

We do this right now with drupalvm with a gulp (or grunt, pick your poison) task that get's fired off in a post provision script. Just make sure that nodejs is installed, then add a package.json and run: node_modules\.bin\gulp
package.json:
{
  "name": "mtech_custom_theme",
  "version": "1.0.0",
  "description": "Theme for Mtech Custom",
  "main": "gulpfile.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "MTech LLC",
  "license": "ISC",
  "devDependencies": {
    "gulp": "^3.9.1",
    "gulp-sass": "^2.3.2"
  }
}

gulpfile.js
var gulp = require('gulp');
var sass = require('gulp-sass');

gulp.task('sass', function () {
	return gulp.src('./assets/sass/**/*.scss')
		.pipe(sass())
		.pipe(gulp.dest('./css'));
});

gulp.task('watch', function () {
	gulp.watch('./assets/sass/**/*.scss', ['sass']);
});

gulp.task('default', ['watch']);

I think there's enough information in this thread to get someone moving in the right direction—thanks for all the help everyone!


Update Absinthe GraphiQL example docs

Hello!
I tried to use the examples in the current doc, but It did not work. So I'm opening this pull request with the solution I found for my problem. I hope that helps.

Were you using a phoenix router or a plug router?

Phoenix router


Enumerable public methods definition

The following methods where defined inside of 'Enumerable' module:

my_each
my_each_with_index
my_select
my_all?
my_any?
my_none?
my_count
my_map
my_inject

Each of them with the purpose of being the own version of Ruby's Enumerable module
Also was defined 'multiply_els' method for testing purposes on 'my_inject'

Every method was tested against the code on 'test.rb' file, wich due to microverse requirements was added to .gitignore

Changes made:
Refactoring of thenext methods:

my_each
my_each_with_index
my_select
my_all?
my_any?
my_none?
my_count
my_map
my_inject

Thanks :)

Status :recycle: [!! Required Changes !!] :recycle:

Hi @ricardovaltierra :wave: ,
massive update :wave: . You have covered most of the task mentioned my the previous reviewer. But still there are some fields that you need to cover to complete this project. And I am sure you will cover all of them.

Please follow the instructions below :point_down: to complete this project.

:muscle: [Mandatory Changes] :muscle:

my_each -> Returns an Enumerator if no block is given
.
For an example I have modified your my_each
https://github.com/ricardovaltierra/enumerable-methods/blob/4a2d9fc2d38697928e44f56767630ef159576629/enumerable.rb#L5-L14
in this way :point_down:

def my_each
    return to_enum(:enum) unless block_given?
    temp = self
    i = 0
    while i < temp.length
      yield(temp[i])
      i += 1
    end
    temp
end

** my_each_with_index** -> Returns an enumerator if no block is given
Check this link. The way to cover it is in :point_up: .
my_select->Returns an enumerator if no block is given. Also same as above.
my_all -> Returns true if all of the collection is a member of such class. Check this link
For an example, [1, 2, 3].my_all?(Integer) should return true.
my_map ->Returns an enumerator if no block is given. Also same as above.

:clubs: [Optional Changes] :clubs:

I have tried to refactor your my_all? and came up with this. If you think this is way more short then your one, you can apply the way i have used.
def my_all?(pattern = nil)
    if pattern # If a pattern is given
      my_each { |element| return false unless pattern === element }
    elsif block_given? # If a block is given
      my_each { |element|  return false unless yield(element) }
    else # If nothing is given
      my_each { |element| return false unless element }
    end
    true
 end

Please fix the above mentioned issue and send another review request.

:herb: Happy Coding :herb:
Shakhawat Hossain
Linkedin | [Slack](Shakhawat Hossain)

The following modifications were made:

my_each -> Returns an Enumerator if no block is given
my_each_with_index -> Returns an enumerator if no block is given
my_select -> Returns an enumerator if no block is given
my_all -> Returns true if all of the collection is a member of such class. * Test made with [1, 2, 3].my_all?(Integer)
my_map -> Returns an enumerator if no block is given. Also same as above.
my_all? -> Refactored with recomended code to improve performance

The following changes were made:
-Returns an Enumerator if no block is given (line 85)


Inaccuracy in data privacy note for version 2.1

Where to find the issue
In the data privacy note for version 2.1, introduced with this PR (iOS).
Describe the issue
The PR https://github.com/corona-warn-app/cwa-app-ios/pull/2457 changed the privacy note to say that iOS 12.5 was the iOS version which introduced the ENF. However, that's not true since Apple only back ported the ENF to iOS 12.5, but iOS 13.0 - 13.4 never had the ENF.
See also https://www.coronawarn.app/de/faq/#ios135.
Suggested change
Change the privacy note to make clear that iOS 13.0 - 13.4 didn't ever had the ENF.

The app store description of CWA says: "Requires iOS 12.5 or later". The CWA terms of use say: "The App uses the COVID-19 Exposure Notification System implemented by Apple in iOS and Google in Android. This is only available in iOS version 12.5 and upwards [...]".
Both descriptions are not precise wrt. 13.0 – 13.4, but this is a minor inaccuracy.

Okay, agreed. Closing this. Thanks @tcfos!


Potential Bug with using option('...') in combination with -p flag.

Q
A

Issue Type
Bug

Deployer Version
3.3.0

Local Machine OS
Centos 7

Remote Machine OS
Centos 7

Description
When I add an option, then use that option, and combine it with -p it seems to silently die.
Steps to reproduce

Add option('dry-run', null, null, 'Use rsync --dry-run.');

Use two or more servers.

Run dep test dev --dry-run -p -vvv

Observe it lists...
➤ Executing task test
↳ on [server1]
↳ on [server2]

But never gets past that step.
Thoughts?

After further testing, it appears that when setting -p it automatically sets the option to 1. Could be related.

Ah, I think this is the problem:
https://github.com/deployphp/deployer/blob/master/src/Executor/ParallelExecutor.php#L201
This looks like it's force adding my options, but I'm using flags, so there are no values provided.
My hack-around is to use InputOption::VALUE_REQUIRED with a default value of 0, and then call --dry-run=1 to enable it.
This bug should be fixed so userland can create flags like -r or --dry-run without requiring values. Sorry I don't know how to fix it.

We need to separately collect valueless options and pass it through. I'll fix it.

I made it! Done in v4!


Fails compilation with emscripten if host platfrom is Mac

In ca3f87560a0eef716195cadf66dc6b938a579ec6 the following lines were added
https://github.com/abseil/abseil-cpp/blob/master/absl/time/internal/cctz/BUILD.bazel#L80-L82
This causes emscripten builds that execute on Mac as host platform to fail with
shared:ERROR: Foundation: No such file or directory ("Foundation" was expected to be an input file, based on the commandline arguments provided)

This makes sense, as emscripten does not have OSX Foundation libraries.
To the best of my knowledge CROSSTOOL in bazel, while allowing to change cpu does not allow to change os, thus I'm not aware of the workaround without modifying abseil source.

Defining cc_target_os in the CROSSTOOL doesn't fix the issue.


Support for coffeescript 2

Coffeescript 2 changed the import from coffee-script to coffeescript.
I made a pull request to support both: https://github.com/sequelize/umzug/pull/158

Hi! Could you do a new release with this PR?
Thanks

This is released as of v2.2.0. However the plan is to drop built-in coffeescript support - to use it in v3 you'll need a custom resolver.


Isolated Process Azure function v4 with .net 6 fails on AKS (linux)

We have multiple function apps running fine with .net core 3.1 and currently hosted in AKS (linux). We are moving all of the apps to .net 6 and func v4. While this works locally pretty good, its not same once deployed to azure environment with below error. While we dont have any direct dependency on the framework '' but we do have multiple asp.net related packages
error
info: Host.Startup[314]
      Loading functions metadata
info: Host.Startup[315]
      8 functions loaded
info: Host.Startup[0]
      Generating 8 job function(s)
info: Host.Function.Console[0]
      It was not possible to find any compatible framework version
info: Host.Function.Console[0]
      The framework 'Microsoft.AspNetCore.App', version '6.0.0' (x64) was not found.
info: Host.Function.Console[0]
        - No frameworks were found.
info: Host.Function.Console[0]
info: Host.Function.Console[0]
      You can resolve the problem by installing the specified framework and/or SDK.
info: Host.Function.Console[0]
info: Host.Function.Console[0]
      The specified framework can be found at:
info: Host.Function.Console[0]
        - https://aka.ms/dotnet-core-applaunch?framework=Microsoft.AspNetCore.App&framework_version=6.0.0&arch=x64&rid=debian.11-x64
// DELETED Messages //
info: Host.Startup[413]
      Host started (959ms)
info: Host.Startup[0]
      Job host started
Hosting environment: Production
Content root path: /home/site/wwwroot
Now listening on: http://[::]:80
Application started. Press Ctrl+C to shut down.
info: Host.General[337]
      Host lock lease acquired by instance ID '00000000000000000000000077302ED5'.
fail: Microsoft.Azure.WebJobs.Script.Description.WorkerFunctionInvoker[0]
      Final functionDispatcher state: WorkerProcessRestarting. Initialization timed out and host is shutting down
fail: Microsoft.Azure.WebJobs.Script.Description.WorkerFunctionInvoker[0]
      Final functionDispatcher state: WorkerProcessRestarting. Initialization timed out and host is shutting down
info: Microsoft.Azure.WebJobs.Host.DrainModeManager[0]
      DrainMode mode enabled
info: Microsoft.Azure.WebJobs.Host.DrainModeManager[0]
      Requesting cancellation for function invocation 'c5a5fd42-a17f-4893-9d20-21859e1a21ec'
info: Microsoft.Azure.WebJobs.Host.DrainModeManager[0]
      Requesting cancellation for function invocation '6e447eb8-08ec-4d6b-92f3-e958c6ceeb90'
info: Microsoft.Azure.WebJobs.Host.DrainModeManager[0]
      Requesting cancellation for function invocation '6a07fa27-d2c8-4dac-9bf0-4a5711b64b93'
info: Microsoft.Azure.WebJobs.Host.DrainModeManager[0]
      Requesting cancellation for function invocation '10a87052-fb25-4480-a30e-e8b6a95e881d'
info: Microsoft.Azure.WebJobs.Host.DrainModeManager[0]
      Calling StopAsync on the registered listeners

This is the csproj
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <RuntimeIdentifiers>win-x86;win-x64;win-arm;linux-x64;linux-arm</RuntimeIdentifiers>
    <TargetFramework>net6.0</TargetFramework>
    <AzureFunctionsVersion>v4</AzureFunctionsVersion>
    <UserSecretsId>r98c7709-6050-4530-bd33-ed0et8b1b448</UserSecretsId>
    <OutputType>Exe</OutputType>
    <_FunctionsSkipCleanOutput>true</_FunctionsSkipCleanOutput>
    <DockerFastModeProjectMountDirectory>/home/site/wwwroot</DockerFastModeProjectMountDirectory>
    <DockerDefaultTargetOS>Linux</DockerDefaultTargetOS>
    <DockerfileContext>..\..</DockerfileContext>
  </PropertyGroup>

  <ItemGroup>
    // some business related projects referred here, all targeting either .net 6 or .net core 3.1
  </ItemGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Azure.Functions.Worker.Sdk" Version="1.3.0" OutputItemType="Analyzer" />
    <PackageReference Include="Microsoft.VisualStudio.Azure.Containers.Tools.Targets" Version="1.14.0" />

    <PackageReference Include="AutoMapper" Version="10.1.1" />
    <PackageReference Include="Microsoft.AspNetCore.Http" Version="2.2.2" />
    <PackageReference Include="Azure.Identity" Version="1.5.0" />
    <PackageReference Include="AutoMapper.Extensions.Microsoft.DependencyInjection" Version="8.1.1" />
    <PackageReference Include="Microsoft.Extensions.Configuration.UserSecrets" Version="6.0.0" />
    <PackageReference Include="Microsoft.Extensions.Http.Polly" Version="6.0.0" />
    <PackageReference Include="Azure.Extensions.AspNetCore.Configuration.Secrets" Version="1.2.1" />
    <PackageReference Include="Azure.Messaging.EventHubs" Version="5.6.2" />
    <PackageReference Include="System.Text.Encodings.Web" Version="6.0.0" />
    <PackageReference Include="Polly" Version="7.2.2" />

    <PackageReference Include="Microsoft.Azure.Functions.Worker" Version="1.6.0" />
    <PackageReference Include="Microsoft.Azure.EventGrid" Version="3.2.1" />
    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.EventHubs" Version="4.3.0" />
    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.ServiceBus" Version="4.2.1" />
    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.CosmosDB" Version="3.0.9" />
    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Storage" Version="4.0.4" />
    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Timer" Version="4.1.0" />
  </ItemGroup>

  <ItemGroup>
    <None Update="host.json">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </None>
    <None Update="local.settings.json">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <CopyToPublishDirectory>Never</CopyToPublishDirectory>
    </None>
  </ItemGroup>
</Project>

ADO command to package the app is dotnet publish -o /SomeOutputPath -c release (using linux agents)
docker file format is below (where we copy from already published folder 'SomeOutputPath' (this is to avoid nuget restore auth etc inside docker build for now)
FROM mcr.microsoft.com/azure-functions/dotnet-isolated:4-dotnet-isolated6.0 AS base
WORKDIR /home/site/wwwroot
EXPOSE 80

COPY SomeOutputPath .

ENV AzureWebJobsScriptRoot=/home/site/wwwroot \
    AzureFunctionsJobHost__Logging__Console__IsEnabled=true

Even if we use FROM mcr.microsoft.com/dotnet/sdk:6.0 AS base image, its same error.
All the AKS pods have env variable set as FUNCTIONS_WORKER_RUNTIME=dotnet-isolated
Is this a known issue? Any help appreciated.

The Microsoft.AspNetCore.App runtime was not included in the Linux images.  This was fixed last week in this PR: https://github.com/Azure/azure-functions-docker/pull/562.
Give it a few days for the newly built image with this fix to be available. I will post an update when it is available. Thanks!

The Microsoft.AspNetCore.App runtime was not included in the Linux images. This was fixed last week in this PR: Azure/azure-functions-docker#562.
Give it a few days for the newly built image with this fix to be available. I will post an update when it is available. Thanks!

Cool. Thanks for the update.

Closing this as answered, but please let us know if you have any other issues.

@fabiocav @kshyju Do we know if the images are updated? Or how can I check if the PR is merged & released already?

@sanjaydebnath Not yet. There was a deployment freeze last week due to holidays. I will post an update in this thread when the deployment is done, and images are available.

Is there an update on this?
We're receiving the same error message when attempting to run our .NET 6.0 (v4 function runtime) function app on a Linux consumption plan.

I was able to get things runnings this weekend:

Create new a function app, couldn't deploy to existing apps even though all settings seem proper.
Update Application Setting FUNCTIONS_WORKER_RUNTIME from dotnet to dotnet-isolated.
Change LinuxFxVersion with az functionapp config set --name <FUNCTION_APP> --resource-group <RESOURCE_GROUP> --linux-fx-version 'DOTNET-ISOLATED|6.0'. https://docs.microsoft.com/en-us/azure/azure-functions/set-runtime-version?tabs=azurecli#manual-version-updates-on-linux
Create new Devops Release pipeline, again couldn't get existing ones to work even though the new and old look identical.

Good luck!

@sanjaydebnath Not yet. There was a deployment freeze last week due to holidays. I will post an update in this thread when the deployment is done, and images are available.

Any update @kshyju ?

@fabiocav @kshyju What is the current status of the images? I still have the same issue as @sanjaydebnath running docker-compose.yml for linux (.NET 6, Isolated Azure Functions v4)

I'm also running into similar problem: @fabiocav @kshyju can we get the new image?

I have the same issue but in the Azure function app.
Any update?
Logs:
2022-01-07T11:56:56.343 [Information] Loading functions metadata
2022-01-07T11:56:56.613 [Information] 5 functions loaded
2022-01-07T11:56:56.674 [Information] Generating 5 job function(s)
2022-01-07T11:56:56.876 [Information] It was not possible to find any compatible framework version
2022-01-07T11:56:56.877 [Information] The framework 'Microsoft.AspNetCore.App', version '6.0.0' (x64) was not found.
2022-01-07T11:56:56.914 [Information] - No frameworks were found.
2022-01-07T11:56:56.915 [Information] You can resolve the problem by installing the specified framework and/or SDK.
2022-01-07T11:56:56.915 [Information] The specified framework can be found at:
2022-01-07T11:56:56.915 [Information] - https://aka.ms/dotnet-core-applaunch?framework=Microsoft.AspNetCore.App&framework_version=6.0.0&arch=x64&rid=debian.11-x64
2022-01-07T11:56:57.187 [Information] It was not possible to find any compatible framework version
2022-01-07T11:56:57.188 [Information] The framework 'Microsoft.AspNetCore.App', version '6.0.0' (x64) was not found.
2022-01-07T11:56:57.188 [Information] - No frameworks were found.
2022-01-07T11:56:57.188 [Information] You can resolve the problem by installing the specified framework and/or SDK.
2022-01-07T11:56:57.188 [Information] The specified framework can be found at:
2022-01-07T11:56:57.188 [Information] - https://aka.ms/dotnet-core-applaunch?framework=Microsoft.AspNetCore.App&framework_version=6.0.0&arch=x64&rid=debian.11-x64
2022-01-07T11:56:57.306 [Information] Found the following

@fabiocav can we please reopen this until actually fixed, answered but not fixed yet.

@sanjaydebnath Sorry! Deployment took more time than originally anticipated. Deployment is currently in progress. I will share an update here when it is completed. Thank you for your patience.

The fix is available in the app service image. You could use that as the base image.
mcr.microsoft.com/azure-functions/dotnet-isolated:4.1.3-dotnet-isolated6.0-appservice
Non app service images are still not ready yet.

3. az functionapp config set --name <FUNCTION_APP> --resource-group <RESOURCE_GROUP> --linux-fx-version 'DOTNET-ISOLATED|6.0'

thank you so much for this! @mark-comeau - this fixed my issue with my dotnet 6 function app not running on my Linux App Service plan.

Non app service images are still not ready yet.

@kshyju is there an update on non app service images?


Collaborator can Edit a Feature File

Opening this to start having all of the interesting conversations this brings up. Pieces of this can likely be broken out into finer grained issues that represent steps toward an eventual goal.
UI

auto complete domain terms
auto complete roles

Collaboration

merge changes from multiple users
give users a collaborative editor (e.g. google docs)

Resources

operational transformation
https://firepad.io/

When viewing a feature, it would be really nice to be able to link to (or rollover) Domain Terms and User Roles as well.

We'll need to support our users' Git workflows which means exposing the ability to create/edit a feature on a branch. This brings up a lot of interesting problems to solve.

when do we pull new changes to the repo?
how to we allow users to create/select a working branch?
how do we enable multiple users to work concurrently on different branches of the same repo?
how do we merge changes made to a branch?*


Prevent warnings when RNAppsFlyerStrictMode is false

When RNAppsFlyerStrictMode is set to false in Podfile, there should be no warning printed.
Also fixes a small copy issue.
Description
There is a warning that is printed anytime this variable is undefined or not true. That is helpful when the variable is undefined, but there's no reason to print the warning if a dev has explicitly set it to false.
Related to https://github.com/invertase/react-native-firebase/pull/6353

Hi, do these new tags mean this will be reviewed?


Issue: Spelling Error in the Augmented Reality Description [GSSoC'23]

The Description:

AR meditation combines ancient teachings of the Gita with AR technology, making it more accessible and engaging for modern practitioners. This unique and immersive meditation experience helps people connect more deeply with their inner self and the divine.

Here, the word self needs to be changed to selves as it is referring to people (plural).

I would like to work on this.

I am interested to work on this issue under GSSoC 2023 as a contributor

Please assign this issue to me. I would like to work on it @akshitagupta15june


bug(help-component): ExpressionChangedAfterItHasBeenCheckedError

Do you want to request a feature or report a bug?
Loading help-component throws the following error:
ERROR Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'id: undefined'. Current value: 'id: mat-dialog-0'. It seems like the view has been created after its parent and its children have been dirty checked. Has it been created in a change detection hook ?

Bug Report

That is a screenshot of the old demo. That error does not show up in the docs.


How to add credentials to httpConnection

I am new with ios, please help how can i add credentials.
Like i have added from android
Platform.loadPlatformComponent(new AndroidPlatformComponent());

Credentials credentials = new Credentials() {
    @Override
    public void prepareRequest(Request request) {
        request.addHeader("User-Name", "BNK");
    }
};

String serverUrl = "http://<IP_ADDRESS>";
mHubConnection = new HubConnection(serverUrl);
mHubConnection.setCredentials(credentials);
String SERVER_HUB_CHAT = "ChatHub";
mHubProxy = mHubConnection.createHubProxy(SERVER_HUB_CHAT);

Looking at your Android code I think this not the client you want to use for iOS. You seem to be using the old (non-Asp.NET Core) version of SignalR and this client only supports the new (i.e. Asp.NET Core) version of SignalR. Asp.NET and Asp.NET Core versions of SignalR are not compatible and you cannot use the new client with the old server or vice versa.
With regards to credentials - this client does not have any higher level support for this. You can set headers directly using HttpConnectionOptions. There is also the accessTokenProvider factory on HttpConnectionOptions that populates the Authorization header with the bearer token in your requests if you configured this kind of authorization on the server side.

Considering the question answered.


Add API for counting all objects in a collection

Goal
It pretty common to want to take a collection of items, and count the number of occurrences of each item. e.g.
let input = ["a", "b", "c", "b", "a"]

let desiredOutput = ["a": 2, "b": 2, "c": 1]

Today
There's 2 relatively short ways to achieve this today:

Using reduce: input.reduce(into: [:]) { $0[default: 0] += 1 }
Reduce is really general, and isn't particularly readable, especially for beginners. The performance here is good though, allocating a single dictionary and mutating it in-place.

Using group(by:): group(by: { $0 }).mapValues(\.count)
We could use the group(by:) helper that I added to Swift Algorithms, but it allocates a ton of intermediate arrays for all the groups, when all we need is their counts.

Proposed solution
The exact name is TBD, but I'm proposing a function like:
extension Sequence where Element: Hashable {
    func tallied() -> [Element: Int] {
        return reduce(into: [:]) { $0[default: 0] += 1 }
    }
}

We could also consider taking a by: parameter, to count things by a value other than themselves. Though perhaps .lazy.map would be better. E.g. input.tallied(by: \.foo) could be expressed as input.lazy.map(\.foo).tallied()
Alternatives
A more general "collectors" API
Similar to Java collectors, which let you express transformations over streams, collecting into Arrays, Dictionaries, Counters, or anything else you might like.
This could pair well with Swift Collections, e.g. if we added a new CountedSet (a native Swift alternative to NSCountedSet. E.g. we could have:
input.grouping(by: \.foo, collectingInto: { CountedSet() })

Prior art

Language
Name

Python
collections.Counter

Ruby
tally

Java
java.util.stream.Collectors.counting()

JavaScript (Lodash)
countBy

C#, Rust don't have helpers for this.

Would this API be just a different spelling for a CountedSet initializer, or would there be meaningful differences?

@xwu Oh cool, I didn't notice there was already an issue for a counted set.
Even if it did exist, I think we should expose a method on Sequence for producing it. For the same reason why input.grouped(by: ...) is nicer than Dictionary(grouping: input, by: ...) (see rationale and PR). In that case, that would make more sense to live in Swift Collections.


PowerShell guide for converting downloaded JSON formatted Risk Detections to CSV format (is better than just downloading CSV directly)

When you download Risk Detections in CSV format, the CSV is very slimmed down.  When you download the JSON format, the file is very difficult to transform in Excel.  So, I'm opening this issue just to share this PowerShell snippet to get the high-fidelity CSV which is best of both worlds.
$ConvertedJson = ConvertFrom-Json (Get-Content "$Home\Downloads\Risk Detections.json")

$CsvFromJson = $ConvertedJson |
Select-Object -Property id,
correlationId,
riskType,
riskState,
riskLevel,
riskDetail,
source,
detectionTimingType,
activity,
ipAddress,
@{
    Name='location.City';
    Expression = {$_.location.City}
},
@{
    Name='location.State';
    Expression = {$_.location.State}
},
@{
    Name='location.Country';
    Expression = {$_.location.Country}
},
activityDateTime,
detectedDateTime,
lastUpdatedDateTime,
userId,
userDisplayName,
userPrincipalName,
@{
    Name = 'additionalInfo.Field'
    Expression = { $_.additionalInfo.Field }
},
@{
    Name = 'additionalInfo.Value'
    Expression = { $_.additionalInfo.Value }
},
tokenIssuerType

$CsvFromJson | Export-Csv $Home\Desktop\RiskDetections.csv -NTI

This gives you all properties, except - I've omitted CountryOrRegion from within additionalInfo, because I've found Country is good enough as is.
Gist

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

ID: 198f0e69-af2a-8c82-041a-c5ca4126f02f
Version Independent ID: 4be57788-f1c2-d6d4-b94a-a59350a618a7
Content: Azure Active Directory risk detections
Content Source: articles/active-directory/reports-monitoring/concept-risk-events.md
Service: active-directory
Sub-service: report-monitor
GitHub Login: @MarkusVi
Microsoft Alias: markvi

@JeremyTBradshaw Thanks for sharing the script !  I have assigned this issue to content author to investigate and update the document as appropriate.
@MarkusVi  can you please help checking if this can be added to the existing documentation.

@SaurabhSharma-MSFT - there are no plans to add code snippets to this article at this point.

Thanks and it's ok.


Update names.txt

I added my full name as instructed by the tutor.

Please I am sorry for all the trouble we have caused you but count it all joy. Your PR is about to be merged and you will be able to see your name on this url.
To enable me to do this, READ the README again carefully, update your code with the latest and push the update. I am here for you for the next 6 hours before I close the PR so you can initiate a fresh PR which will be easier to merge.

Good evening, thank you so much for your patience and all the work, it must be alot. I'm just seeing your mail now and I'm just leaving the office because I'm just closing from work now. Today was a busy day for me. I don't even I can still meet up with the github live class work but if not I will read to know and understand it when I get home.
Thank you and have a good evening ahead.
Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Olúwásetèmi @.>
Sent: Monday, August 15, 2022 5:36:20 PM
To: Oluwasetemi/altschool-opensource-names @.>
Cc: Juliana Micheal-Echezona @.>; Author @.>
Subject: Re: [Oluwasetemi/altschool-opensource-names] Update names.txt (PR #497)
Please I am sorry for all the trouble we have caused you but count it all joy. Your PR is about to be merged and you will be able to see your name on this urlhttps://altschool-opensource-names.oluwasetemi.dev/.
To enable me to do this, READ the README again carefully, update your code with the latest and push the update. I am here for you for the next 6 hours before I close the PR so you can initiate a fresh PR which will be easier to merge.
—
Reply to this email directly, view it on GitHubhttps://github.com/Oluwasetemi/altschool-opensource-names/pull/497#issuecomment-1215322671, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AVIUTJ4HUKOZ3D6SDWGGFOLVZJWYJANCNFSM56SGJVOA.
You are receiving this because you authored the thread.Message ID: @.***>


Using commas in Checkbox option text

When building a form with form-builder, whats the workaround for using a comma inside one option's text, lets say, for a "Accept conditions" checkbox, there is usually a long text which may include commas. But if commas are used, they are taken as a new options, which is wrong.

As part of the Wagtail’s first Kickstarter campaign, I’m working on this!
https://www.kickstarter.com/projects/noripyt/wagtails-first-hatch

I think the newline approach could be good, means this field is a text area instead of a single line.
However, we need to think of existing implementations that use commas.
The backwards compatible approach would be to allow both to exist (as per the implementation above).
Marking this as a good first issue.

@lb- Started working on it. Should I include the feature of separating the value and label with a pipe character?

Nevermind. I'll just focus on the newline approach only.

@amajai good call, newline only is a good way to go for now. We can always add more complexity if needed later. Plus it keeps the scope of the PR simpler.
Remember to add docs, unit tests please.


Integrate PVC creation feature using snapshot to replication controller for single cluster

Description

This feature extends the existing repctl / replication controller from just creating snapshots to also creating PVCs necessary to start a test copy of the application. For single cluster the test copy of the application must be started in a different namespace than the production copy of the application which is assumed to be running.
This uses the replication action to take a snapshot of the target volume(s), and is applied to all volumes in the replication group
PVC/PV pairs are created in the namespace of the VolumeSnapshots taken
The user can start a new copy of the original application using the replicated objects by specifying the new namespace of the created PVC/PV pairs

GitHub Issues
List the GitHub issues impacted by this PR:

GitHub Issue #

N/A

Checklist:

[x] I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
[x] I have verified that new and existing unit tests pass locally with my changes
[x] I have not allowed coverage numbers to degenerate
[ ] I have maintained at least 90% code coverage
[x] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[x] I have added tests that prove my fix is effective or that my feature works
[x] I have maintained backward compatibility

How Has This Been Tested?

[x] Manual test using replication action
[x] Unit testing
[ ] Integrated testing

Manual Testing Instruction

Rebuild repctl
The user has the option to specify the snapshot class in the replication action they provide: repctl snapshot --rg rg-id --sn-class vxflexos-snapclass-delete --sn-namespace test-delete --create-pvcs --storage-class vxflexos-217
The user also has the option not to specify a snapshot class in the replication action, and a sufficient default class will automatically be supplied or created if it does not already exist: repctl snapshot --rg rg-id --sn-namespace test-default --create-pvcs --storage-class vxflexos-217
Deploy an app, but update helm chat to specify namespace of newly created PVC/PV (e.g. test-pg1)

Test Coverage Change
/controllers/replication-controller/dellcsireplicationgroup_controller.go change:

Original test coverage of main branch
Latest test coverage due to added code

58.4%
76.4%

Is this PR ready for review? It's in draft state.

Moving to Draft state, need to check with POs regarding this feature's milestone.


Same pragma compiler version

All sol files except BurnableToken.sol and BurnableTokenMock.sol declares pragma compiler ^0.4.11. The latter two files use pragma compiler ^0.4.13.
Maybe it makes sense to use same pragma compiler in all files?

Included in https://github.com/OpenZeppelin/zeppelin-solidity/issues/511


Update 0494-target-sum.py (runtime and memory > 80%)

File(s) Modified: 0494-target-sum.py
Language(s) Used: _python
Submission URL: https://leetcode.com/problems/target-sum/submissions/905306462/
optimize more by:

using only 2 rows of dp
iterate only on non zero previous row dp

Same idea but using a Counter makes it a bit more readable, in my opinion, and the performance is similar.
Runtime 304 ms Beats 87.41%
Memory 14.1 MB Beats 88.17%
https://leetcode.com/problems/target-sum/submissions/842843238/
class Solution:
    def findTargetSumWays(self, nums: List[int], target: int) -> int:
        dp = Counter([0])
        for num in nums:
            next = Counter()
            for key in dp.keys():
                next[key - num] += dp[key]
                next[key + num] += dp[key]
            dp = next
        return dp[target]


Binding text in HamburgerButtonInfo

Hi
I have a question about text binding in HamburgerButtonInfo. Is this even possible?
I tried with {Binding DataContext.ButtonText, ElementName='ShellPage'}
but it doesnt work for me
thanks

solved


Handle redirects

There are some situations were redirects are encouraged, specifically:

If two URIs differ only in the trailing slash, and the server has associated a resource with one of them, then the other URI MUST NOT correspond to another resource. Instead, the server MAY respond to requests for the latter URI with a 301 redirect to the former.

We could do this by throwing errors with 3xx status codes, but since these are not actual errors this is sort of wrong. On the other hand, this might be requested by functions that don't actually return anything (e.g., setRepresentation), so they have no real way of returning anything in another way.
Related to #11 and #86 .

This comment pertains specifically to

If two URIs differ only in the trailing slash, and the server has associated a resource with one of them, then the other URI MUST NOT correspond to another resource. Instead, the server MAY respond to requests for the latter URI with a 301 redirect to the former.

Assuming we probably do want to do the redirect.
If we don't want to change the interface of ResourceStore, I can only see a few possible ways to go about this:

Have the GetOperationHandler (Head/Post/Put/etc.) handle this. It could do an additional request to the store with a differing trailing slash if the first request resulted in a 404. This would increase the amount of requests that is done in case of every 404 though.
In the case of GET (and HEAD), we could have the store always return the result independent of trailing slash, and then have the operation handler check if the input identifier matches the resource identifier. If not a redirect can be done.
As mentioned in the initial comment, they could be handled the same as errors, even though they're not. This would also require an extra field in the "error" object to contain the redirect URL.
The ResourceStore adds a field to the RepresentationMetadata indicating the status code that should be used (but also only works for GET/HEAD since only there metadata gets returned).

If we do change the interface of ResourceStore, some solutions include

Having all functions return a "Status" object next to their other return value that corresponds to the response status code (similarly to how we handle errors now). Potentially those objects then contain a function that can add headers to a server response (e.g., link headers for 201 and 301).
Add a function that returns a boolean if a resource exists in the store or not (or only returns the metadata). This would decrease the cost in case we let the operation handlers do multiple requests to check if a redirect exists.

Note that it is recommended to only return 301 for GET/HEAD (and 308 for POST) so there is something to be said for the solutions that would only work with GET/HEAD.

A more extensive discussion/study of how to handle return status codes and additional headers. Also related to #11, #233, #264.  Actually not sure if an issue is the best place to put this. 😅
I'm actually still not sure what the best solution would be but I'm posting it here to potentially have some more eyes on it and to let it rest for now.
Some points:

#233 has an important impact on this issue depending on how we solve it. The solution that makes most sense to me is to have a ResourceStore with a map of redirects. And if the input identifier matches a key it "returns" the redirect to the value. (More on how to return this redirect later).
Every OperationHandler should return a status code.
Some status codes correspond to extra headers (201 and 301 for example, potentially also for certain errors?).
The metadata object in a Representation will be (partially) converted to response headers (#264).

There are several points related to all this so it's going to be a bit of a combination of stuff.
Rename RepresentationMetadata to Metadata
The name RepresentationMetadata is a misnomer for how we currently use it. Whether a resource is a container, or what the slug is, are not related to the Representation. All of the metadata we get from the extractor are headers that got converted so might as well call it RequestMetadata. At the resource store level these then get (partially) converted to metadata related to the resource (ResourceMetadata), and for the response we're also looking to return headers based on the metadata (ResponseMetadata). So I would just rename it to Metadata to have a general cover-all.
Decouple Metadata from the Representation
Since the Metadata is only partially relevant to the Representation, I would decouple it from the Representation and have it as a separate parameter where both are needed. The disadvantages are that the addResource and setRepresentation functions would require 4 parameters (identifier, representation, metadata, conditions), and the getRepresentation would still need to return an object containing both the representation and the metadata.
Handle redirects
Following from the #233 comment, redirects need to be handled on ResourceStore level (unless we go with a different solution there of course). Simply having an OperationHandler do 2 calls would not be sufficient if we also have a redirect map. So the only 2 options I see here are having all ResourceStore operations return a metadata object (which would then also include metadata about the operation), or throw an error which also includes the redirect URL.
In favor of errors
Interpreting redirects as errors would be the easiest solution, requiring the least changes in the ResourceStore code. We could then have a RedirectHttpError which takes as input an URL to redirect to. The one thing that is missing is how to extract that metadata from the error when writing the response. One solution there would be to give HttpErrors a new function that allows them to write headers to a Response object.
We already have the error solution in place (and I'm actually quite happy how easy it makes to break the flow when something goes wrong/is missing), so it would not be that much extra to add redirect errors.
Currently the only need for returning metadata would be redirecting, so stores are going to return empty metadata most of the time just for this specific case. In case of PATCH/PUT/DELETE that is, there is something to be said for adding the resulting identifier to the metadata when doing a POST (and of course a GET would always result in metadata).
In favor of metadata
The advantage of returning metadata instead is that we already are planning to convert metadata to headers, so this metadata would just be included there. This would also make it more consistent and put all the extra header information in 1 object.
Later on we might want to add even more metadata to the output which would be impossible to do on ResourceStore level since there is no output value there for certain functions. Some examples are a link header linking to the acl file or which input/output types are supported. There are only a few locations where we would be able to put these: a nested ResourceStore, the OperationHandlers, the ResponseWriter, or the main AuthenticatedLdpHandler. The last 3 would need a new (composite) handler to add metadata to the response.
Status codes
Also related is how we return the different 2xx status codes to the ResponseWriter but this is less of an issue since there are very few different cases. GET should always have 200, POST 201, PUT/PATCH 205 and DELETE 200. So we could add an extra field in the ResponseDescription object and let the OperationHandlers do this. Something somewhere would have to know to overwrite this if a 301 is required though (so this is another point in favor of using errors for redirects since errors bypass this). Or when going for the metadata solution it's also possible that the store adds a metadata entry corresponding to the status code (e.g., <id> <operationResult> <ok>.), but that's probably overkill.

General opinion
In general, I'm in favor of the error solution. I don't consider this an abuse, in the sense that it's an exception: the normal flow is to just get the thing, but now the thing is somewhere else, so we throw.
Metadata
However, if non-2xx responses suddenly would also need to include headers… then we'd need to attach the metadata to the error. And that's the point where we might be going wrong.
About the specific point of status codes

GET should always have 200, POST 201, PUT/PATCH 205 and DELETE 200. So we could add an extra field in the ResponseDescription object and let the OperationHandlers do this.

That was definitely my intention.
About the specific point of the metadata misnomer

The name RepresentationMetadata is a misnomer for how we currently use it. Whether a resource is a container, or what the slug is, are not related to the Representation.

I disagree here. A representation always corresponds to exactly one resource, so hence, metadata about the representation also corresponds to exactly one piece of metadata about the resource. So it's really just transitivity; yes, the fact that X is a representation of a container is because its corresponding resource is a container. Yet I see no strong reason at the moment to separate representation metadata from resource metadata, given that HTTP/REST ultimately exchanges representations, so what matters is that metadata we are attaching to that representation and its envelope.
Also, the counterexample is contentType, so some (but not all) properties in there pertain only to a specific representation.

All of the metadata we get from the extractor are headers that got converted so might as well call it RequestMetadata.

Representations flow both ways; getRepresentation also returns them.

Conclusion
Given the above, my personal conclusion is:

Stores throw an error if the normal flow cannot be followed (so, redirect)
The operation can catch the error, and if headers are required, go back to the store for metadata
We put all of that in the ResponseDescription

I agree with using errors for redirects. Also thought some more on it and it is the one that makes the most sense (and makes several things much easier).
Also most metadata we would need would be in case of doing a GET (/HEAD) and that one returns metadata anyway.

Added in #1064


Blog app - integration specs for views and fix n+1 problems

Description:
This PR addresses the N+1 problem in fetching all posts and their comments for a user. It also includes integration tests for each view in the project.
Changes:

N+1 Problem Optimization: Implemented a solution to the N+1 problem when fetching all posts and their comments for a user. The solution does not use the bullet gem; instead, it analyzes the logs to identify and solve the problem. Screenshots of the app's logs before and after the fix are included in the PR.

Integration Tests: Used Capybara to write integration tests for each view in the project. The tests are grouped in a way that makes sense, with separate tests for checking the elements on the page and for clicking links on it.

User Index Page: Tests to ensure that the username, profile picture, and number of posts for each user are displayed. Also, tests the redirection to the user's show page when a user is clicked.

User Show Page: Tests to ensure that the user's profile picture, username, number of posts, bio, and first 3 posts are displayed. Also, tests the redirection to the user's post's index page when the 'see all posts' button is clicked, and to the post's show page when a post is clicked.

User Post Index Page: Tests to ensure that the user's profile picture, username, number of posts, post's title, some of the post's body, the first comments on a post, the number of comments a post has, the number of likes a post has, and a section for pagination are displayed. Also, tests the redirection to the post's show page when a post is clicked.

Post Show Page: Tests to ensure that the post's title, the author of the post, the number of comments it has, the number of likes it has, the post body, the username of each commentor, and the comment each commentor left are displayed.

N + 1 Problem
User

In the user index (home), we only need the user list and as such only one query (No N + 1 problem) was run as shown below:

In the user show (user profile), we only need the user details and the list of recent posts with not comments and as such only two queries (No N + 1 problem) was run as shown below:

Post

The post index, we have a multiple call to the comment table (N + 1 problem) as shown in the screenshot below:

After solving the N + 1 Problem:

The Post show, we have similar N + 1 problem as shown below:

Invalid Code Review Request ❌
Hi @fmanimashaun ,
While you made a great effort in this project, unfortunately, I cannot proceed to review your code.
You have submitted a project, that is a pair-programming activity and there are no commits from your partner. Please add commits equally or attach a screenshot of letting you work solo from student success
Your Code Review Request will be marked as invalid in your Dashboard, so please submit a new one once you are ready 🙏
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.

Invalid Code Review Request does not count into the code reviews limit.