Insert some comment when writing csv files

Hello,
Thank for this nice library :)
I have trouble to include some comment when writing csv files. Does the library provide a way to do it ?

Thanks @flagarde! Could you share some example on what type of comment you mean?
Is it first row(s) comments, like:
# Some comment about
# the data
ColumnLabel1,AnotherColumn
2,3
5,6

Or perhaps comment at the end of each line, like:
ColumnLabel1,AnotherColumn,Comment
2,3,Comment about this line
5,6,Another commented line

Or perhaps something else?
There's no specific built-in feature for writing comments today, but it's possible existing functionality could be used in some way.

Hello,
Sure . I'm using your library to write csv coming from some data analysis. We are playing with a lot of parameters so I would like to save the command line as comment int the first line. For example :
# ./Analysis -s 210 450 -n 0 150  -c 2 -d 0 0 0 0 0 0 0 0 1 1 1 1 1 1 1 1 -p -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 --noiseAfter 600 1000 --sigmaNoise 5. --sigma 8 --path /run/media/rpclab/1a6b6565-e3a2-41c1-a7b2-6af56a3352d1/data/shifted/ --files 5800V.root 5900V.root 6000V.root 6100V.root 6200V.root 6300V.root 6400V.root 6500V.root 6600V.root 6700V.root 6800V.root 6900V.root --analysed 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 --saveAs ShiftedChamber_8sigmas -e 1000
HV,Efficiency,"Error Efficiency","Efficiency Corrected","Error Efficiency Corrected",Multiplicity
5800,0.005,0.00223047,0.005005,0.0022327,1
5900,0.007,0.00263648,0.00702106,0.00264438,1
6000,0.032,0.00556561,0.032032,0.00557109,1
6100,0.106,0.00973468,0.106,0.00973468,1.06604
6200,0.228,0.0132671,0.228228,0.0132784,1.04825
6300,0.47,0.0157829,0.46994,0.0157986,1.08723
6400,0.638,0.0151972,0.638277,0.0152099,1.0815
6500,0.809,0.0124306,0.809,0.0124306,1.10136
6600,0.887,0.0100115,0.887,0.0100115,1.18489
6700,0.93,0.00806846,0.92993,0.00807623,1.16344
6800,0.952,0.00675988,0.951952,0.00676648,1.23214
6900,0.96,0.00619678,0.95988,0.00621503,1.29896

Best regards

Hi @flagarde - thanks for providing more details about the use-case, and sorry about the late reply. Rapidcsv can support writing such files, but it requires you to "manually" manage the column labels, and to insert the comment line last.
Example creating a CSV file from scratch:
  rapidcsv::Document doc("",
                         rapidcsv::LabelParams(-1 /* no column labels */, -1 /* no row labels*/),
                         rapidcsv::SeparatorParams(',', false, rapidcsv::sPlatformHasCR, false, false));
  doc.SetRow<std::string>(0, std::vector<std::string>({ "HV", "Efficiency" }));
  doc.SetRow<float>(1, std::vector<float>({ 5800, 0.005 }));
  doc.SetRow<float>(2, std::vector<float>({ 5900, 0.007 }));
  doc.SetRow<float>(3, std::vector<float>({ 6000, 0.032 }));
  doc.InsertRow<std::string>(0, std::vector<std::string>({ "# First line comment in CSV" }));
  doc.Save("new.csv");

The resulting file will be:
# First line comment in CSV
HV,Efficiency
5800,0.005
5900,0.007
6000,0.032

Another use-case may be starting with an existing file, example orig.csv content:
HV,Efficiency
5800,0.005
5900,0.007
6000,0.032

Then one can do:
  rapidcsv::Document doc("orig.csv",
                         rapidcsv::LabelParams(-1 /* no column labels */, -1 /* no row labels*/),
                         rapidcsv::SeparatorParams(',', false, rapidcsv::sPlatformHasCR, false, false));
  doc.InsertRow<std::string>(0, std::vector<std::string>({ "# First line comment in CSV" }));
  doc.Save("edited.csv");

The resulting file will be:
# First line comment in CSV
HV,Efficiency
5800,0.005
5900,0.007
6000,0.032

Hope this helps!

I will proceed to close this issue, but feel free to re-open it if you have any follow-up questions.


Check and remove unnecessary shuffle added by Hybrid Scan

What is the context for this pull request?

Tracking Issue: n/a
Parent Issue: n/a.
Dependencies: n/a

What changes were proposed in this pull request?

This PR can remove additional shuffle node by generating executedPlan for the transformed join query plan in optimizer.

get the number of shuffle nodes for left and right plan.
having 2 shuffle nodes means

the candidate index pair cannot remove shuffles for both left and right

Hybrid scan causes unnecessary shuffle to use BucketUnion for appended files

(fyi, there could be max. 3 shuffle nodes in left and right, because if both children of join are bucketed with different number, Spark will shuffle the child with the smaller bucket number of two, using the larger bucket number. For example,

num of buckets for a left candidate index = 11
num of buckets for a right candidate index = 12
Spark will reshuffle the left child using 12 buckets.

In this case, we don't need to shuffle for the appended files, because they will be shuffled for SortMergeJoin after all.
So Hyperspace doesn't use the transformed plan for the child.
But indexes can still be applied for the child using Filter Rule.
Does this PR introduce any user-facing change?

Yes
With this change:
*(6) Project [c2#139, c3#140, c4#151]
+- *(6) SortMergeJoin [c2#139], [c2#149], Inner
   :- *(3) Sort [c2#139 ASC NULLS FIRST], false, 0
   :  +- BucketUnion 12 buckets, bucket columns: [c2]
   :     :- *(1) Project [c2#139, c3#140]
   :     :  +- *(1) Filter isnotnull(c2#139)
   :     :     +- *(1) FileScan parquet [c2#139,c3#140] Batched: true, Format: Parquet, Location: InMemoryFileIndex[file:/path/to/src/test/resources/indexLocation/index2..., PartitionFilters: [], PushedFilters: [IsNotNull(c2)], ReadSchema: struct<c2:string,c3:string>, SelectedBucketsCount: 12 out of 12
   :     +- Exchange hashpartitioning(c2#139, 12)
   :        +- *(2) Project [c2#139, c3#140]
   :           +- *(2) Filter isnotnull(c2#139)
   :              +- *(2) FileScan parquet [c2#139,c3#140] Batched: true, Format: Parquet, Location: InMemoryFileIndex[file:/path/to/Local/Temp/spark-e607f7ad-557a-4980-9eb9-e7284ed..., PartitionFilters: [], PushedFilters: [IsNotNull(c2)], ReadSchema: struct<c2:string,c3:string>
   +- *(5) Sort [c2#149 ASC NULLS FIRST], false, 0
      +- Exchange hashpartitioning(c2#149, 12)
         +- *(4) Project [c2#149, c4#151]
            +- *(4) Filter isnotnull(c2#149)
               +- *(4) FileScan parquet [c2#149,c4#151] Batched: true, Format: Parquet, Location: InMemoryFileIndex[file:/path/to/src/test/resources/indexLocation/index/..., PartitionFilters: [], PushedFilters: [IsNotNull(c2)], ReadSchema: struct<c2:string,c4:int>

Without this change:
*(7) Project [c2#139, c3#140, c4#151]
+- *(7) SortMergeJoin [c2#139], [c2#149], Inner
   :- *(3) Sort [c2#139 ASC NULLS FIRST], false, 0
   :  +- BucketUnion 12 buckets, bucket columns: [c2]
   :     :- *(1) Project [c2#139, c3#140]
   :     :  +- *(1) Filter isnotnull(c2#139)
   :     :     +- *(1) FileScan parquet [c2#139,c3#140] Batched: true, Format: Parquet, Location: InMemoryFileIndex[file:/C:/Users/eunsong/repo/hyperspace2/src/test/resources/indexLocation/index2..., PartitionFilters: [], PushedFilters: [IsNotNull(c2)], ReadSchema: struct<c2:string,c3:string>, SelectedBucketsCount: 12 out of 12
   :     +- Exchange hashpartitioning(c2#139, 12)
   :        +- *(2) Project [c2#139, c3#140]
   :           +- *(2) Filter isnotnull(c2#139)
   :              +- *(2) FileScan parquet [c2#139,c3#140] Batched: true, Format: Parquet, Location: InMemoryFileIndex[file:/C:/Users/eunsong/AppData/Local/Temp/spark-e3245dda-a52a-4c4f-859f-f43f0c9..., PartitionFilters: [], PushedFilters: [IsNotNull(c2)], ReadSchema: struct<c2:string,c3:string>
   +- *(6) Sort [c2#149 ASC NULLS FIRST], false, 0
      +- Exchange hashpartitioning(c2#149, 12)
         +- BucketUnion 11 buckets, bucket columns: [c2]
            :- *(4) Project [c2#149, c4#151]
            :  +- *(4) Filter isnotnull(c2#149)
            :     +- *(4) FileScan parquet [c2#149,c4#151] Batched: true, Format: Parquet, Location: InMemoryFileIndex[file:/C:/Users/eunsong/repo/hyperspace2/src/test/resources/indexLocation/index/..., PartitionFilters: [], PushedFilters: [IsNotNull(c2)], ReadSchema: struct<c2:string,c4:int>, SelectedBucketsCount: 11 out of 11
            +- Exchange hashpartitioning(c2#149, 11)
               +- *(5) Project [c2#149, c4#151]
                  +- *(5) Filter isnotnull(c2#149)
                     +- *(5) FileScan parquet [c2#149,c4#151] Batched: true, Format: Parquet, Location: InMemoryFileIndex[file:/C:/Users/eunsong/AppData/Local/Temp/spark-e3245dda-a52a-4c4f-859f-f43f0c9..., PartitionFilters: [], PushedFilters: [IsNotNull(c2)], ReadSchema: struct<c2:string,c4:int>

How was this patch tested?

Unit test

Measured executePlan performance using TPCH dataset:
val filter1 = linetable.filter(linetable("l_orderkey") isin (1234,12341234, 123456)).select("l_orderkey")
val filter2 = linetable.filter(linetable("l_orderkey") isin (1234,12341234)).select("l_orderkey")
val join = filter1.join(filter2, "l_orderkey")
val plan = join.queryExecution.optimizedPlan
measure(spark.sessionState.executePlan(plan).executedPlan)

val join = filter1.join(filter2, "l_orderkey")
val plan = join.queryExecution.optimizedPlan
measure(spark.sessionState.executePlan(plan).executedPlan)

val join = filter1.join(filter2, "l_orderkey")
val plan = join.queryExecution.optimizedPlan
measure(spark.sessionState.executePlan(plan).executedPlan)

// result (unit: ms)
duration: 233
duration: 97
duration: 97

Measured executePlan performance using TPCH dataset:
val filter1 = linetable.filter(linetable("l_orderkey") isin (1234,12341234, 123456)).select("l_orderkey")
val filter2 = linetable.filter(linetable("l_orderkey") isin (1234,12341234)).select("l_orderkey")
val join = filter1.join(filter2, "l_orderkey")
val plan = join.queryExecution.optimizedPlan
measure(spark.sessionState.executePlan(plan).executedPlan)

val join = filter1.join(filter2, "l_orderkey")
val plan = join.queryExecution.optimizedPlan
measure(spark.sessionState.executePlan(plan).executedPlan)

val join = filter1.join(filter2, "l_orderkey")
val plan = join.queryExecution.optimizedPlan
measure(spark.sessionState.executePlan(plan).executedPlan)

// result (unit: ms)
duration: 233
duration: 97
duration: 97

Measured executePlan performance using TPCH dataset:

Could you explain the importance of this benchmark? (I wasn't sure what to get out of this benchmark.)

Measured executePlan performance using TPCH dataset:

Could you explain the importance of this benchmark? (I wasn't sure what to get out of this benchmark.)

I just checked the code, and you meant to measure re-executing the updated plan?

I just checked the code, and you meant to measure re-executing the updated plan?

Here is what I suggest on how to write the benchmark paragraph (my thoughts in parenthesis):
This PR introduces triggering SparkSession.executePlan inside JoinIndexRule to check if unnecessary shuffle is introduced in the physical plan. Since SparkSession.executePlan may not be a cheap operation (I guess this is why you were doing?), I ran a benchmark on the change.
.. result here ...
I see the results to be 233ms, 97ms, 97ms (explain the reasoning on the high latency on the first iteration - prob. caching), and this doesn't look like introducing much overhead (but I would also compare when SparkSession.executePlan is turned off)

Here is what I suggest on how to write the benchmark paragraph (my thoughts in parenthesis):
This PR introduces triggering SparkSession.executePlan inside JoinIndexRule to check if unnecessary shuffle is introduced in the physical plan. Since SparkSession.executePlan may not be a cheap operation (I guess this is why you were doing?), I ran a benchmark on the change.
.. result here ...
I see the results to be 233ms, 97ms, 97ms (explain the reasoning on the high latency on the first iteration - prob. caching), and this doesn't look like introducing much overhead (but I would also compare when SparkSession.executePlan is turned off)


Unable to create directory

I am using Laravel 9 and I am trying to create a directory in a bucket.
Storage::disk('s3')->makeDirectory("tenants/abcde");

The tenants directory already exists, but running the above command throws the following error.
League\Flysystem\UnableToWriteFile with message 'Unable to write file at location: tenants/abcde/.'

I have checked the credentials that I am using and the access_key has S3FullAccess. The strange thing is, I created a folder directly using the S3 console and I am able to programmatically add files in the folder or even delete the folder, but if I try to make a new directory, I get the above error. I have even tried to use the root access_key and I still get the same error.

On further debugging, I see the following error from AWS.
<?xml version="1.0" encoding="UTF-8"?>
<Error>AccessControlListNotSupported<Message>The bucket does not all (truncated...)
 AccessControlListNotSupported (client): The bucket does not allow ACLs - <?xml version="1.0" encoding="UTF-8"?>
<Error>AccessControlListNotSupported<Message>The bucket does not allow ACLs</Message>

My bucket is public access blocked as recommended by Amazon's best practice and the only way to read files are through cloudfront. There is a bucket policy that allows read access only through cloudfront.
Having said that, debugging the underlying code the following code doesn't work and results in the error above.
use Aws\S3\S3Client;

$client = new S3Client([
    'credentials' => [
        'key'    => env('AWS_ACCESS_KEY_ID'),
        'secret' => env('AWS_SECRET_ACCESS_KEY')
    ],
    'region' => 'us-east-1',
    'version' => 'latest',
]);

$client->putObject(array(
                   'Bucket' => 'bucket-name',
                   'Key'    => "tenant/cdfe",
                   'Body'   => "",
                   'ACL'    => 'public-read'
                  ));

but removing the ACL from the array passed to putObject works correctly.

@abishekrsrikaanth what if you use an ACL of private?

@frankdejonge ACL set to private works.

@abishekrsrikaanth then you can configure the adapter to have a default directory visibility of private to resolve this issue. The visibility constructor property allows you to use a PortableVisibilityConverter instance which accepts this configuration setting.


request:  client pools admin access to initial machines

Hi, I know you did this intentionally for whatever reason is yours but in client_pools.php if you made it so only certain users or admin type users could add initial image machines to specific pools then you would be able to have some granularity in your access.  an admin would have one box in the web client to add software to the guests then they could logout and push right away without ever leaving the browser.  drag and drop works with the client so it would be really useful from an admin perspective or for deploying software.
to clarify the request... the ability to have admin users add initial images to pools... maybe with restrictions that they can only be added to their own pool with no other members.  only admins could also be added to use the pool... "elevated access to control iso via web client" is a good way to phrase.
you probably did this and have some reason for it that i don't know, i expect.

Not sure, whats wrong with current solution - admin is able to install all software via dashboard service?

you can't add initial images to a pool.... it doesn't pop up in the list.  if you did this, it would give the ability to use the html5 client to add software quickly to the image.  shut it down, populate, conveniently.
it also could be a disaster if people didn't understand it... thats why i say for admin type users.

Well, admin users have no ability to use client software and vice versa, so you would have to use second "admin" client login for that.
But if I've understood correctly, you want to have multiple level admins - tjhe ones, who are creating vms and controlling kvm-vdi cluster, and second type, who are just installing software into images and who are denied with operations within cluster.
Then it is really a problem. Perhaps it should be allowed to add Initial//Source images to some kind of pool and provide "admin" user to that pool. Will take a look at this. This will break some policies though. Will have to re-thik that.

yes i believe you understand exactly.


TODO RotateScaleTranslate Listener

Plus an app to test it.

HammerJS May cover all of this (def here).

Or maybe InteactJS.

Revolved by commit 6d1a6a5 - we now have multi-touch gesture support.


Xcode 7.3 Compatibility

As noted in my Eigen PR, Xcode 7.3 doesn't print test results to the debug console like it used to (they're sent to a log file). Snapshots plugin will need to either read that file or (and I think Orta talked to me about this) listen for notifications from Xcode about passed/failed tests.
I'll try to take a stab at this, likely with https://github.com/ashfurrow/second_curtain/issues/40 for good measure.

Probably best to setup a NSFileCoordinator for where-ever the logs come in to, I've wanted to make Snapshots standalone - and this it what it needed to do that. With that there's no need to build into Xcode.

:+1: Cool, thanks for the guidance!

Quick is also looking at something similar, I'll keep an eye on it to see what happens.

scan/xcpretty/quick are all having issues with this. I'm not sure what the solution will be, I've looked through the generated log and plist test summaries and neither has the data as we need it. I'm not really qualified to go digging for which NSNotifications to listen to, so I'm going to have to put this aside for now.

Loooooks like it might've just been a bug in Nimble: https://github.com/Quick/Nimble/pull/271 I'll keep on top of it.

Any news on this? I kinda have a broken CI flow because of this issue (if there's anyway I can help, just let me know).

Yes, the problem has been fixed on Nimble, but the solution is causing new problems. Since the fix is in master, we're using it and it has solved this problem for now.
Actually I think we can close this issue now, since it's not a problem with this specific project. Thoughts?

Aye - I think #37 also covers this from the perspective of doing it in snapshots itself.


Generator expression and comprehension scope

I'm kind of optimistic that this fixes some of https://github.com/cython/cython/issues/1159 and also preserves most of the optimizations made (so range is still translated into a simple for loop, and dict.values etc still works...). I've inspected the code by eye to check the optimizations - I'm not sure if they're tested for in the testsuite or not - so there may be some I've missed but they mostly happen fairly naturally with this approach.
It also works for comprehensions (which is a bit simpler, but important to me because I often like to use locals().items() when debugging, and locals() is now draw from the correct place).
Essentially all I do is change
   [ x for x in something ]

to
tmp = something
[ x for x in tmp ]

at an early stage in the pipeline. There's some added complications because temp variables don't like it when they are defined outside a closure but used inside (so I've added some code to add them into the outer_scope as __pyx_input_0).
I'm pretty certain this isn't perfect, but I thought it was probably in a useful enough state to get feedback for anyway.

@scoder Pinging w.r.t. the alpha release - this issue this addresses is allegedly a "3.0 blocker" (although hopefully the number of people relying on Cython's wrong choice of scope are small)

One of the tests I'd added is currently failing because of https://github.com/cython/cython/pull/3244#issuecomment-603890359... I think the test does a reasonable thing and I don't think here is the place to try to merge a fix.

@da-woods Fixed the issue in PR: #3465. Does it fix your test?

@black-square It doesn't quite but I think it gets it close enough that I can fix the test. I use slices on the array (which isn't supported by ArrayType) but I don't think that's really worth supporting in Shadow.py

@da-woods The actual implementation is in PointerType and I think we can easily add more init cases to it. I personally really into having Pure Mode as close to the "normal" mode as possible. It's the main way for us to keep the code debuggable. Let me know if I can help with that.

@da-woods The actual implementation is in PointerType and I think we can easily add more init cases to it. I personally really into having Pure Mode as close to the "normal" mode as possible. It's the main way for us to keep the code debuggable. Let me know if I can help with that.

So the test in question is https://github.com/cython/cython/blob/43bab6366931005be5620be5f1e8ae955c149af9/tests/run/genexpr_arg_order.py#L76-L77 (and one other). Essentially what I'm trying to do is prove that f() and g() are evaluated in the same order in both Python and Cython (hence pure-Python mode).
The __init__ is now working fine. What's now breaking is __getitem__ https://github.com/cython/cython/blob/48dc1f0169f81ed20c6b374941c2498ec94e57d0/Cython/Shadow.py#L249-L252 because it assumes that you can only pass an int, while Cython does let you use slicing (in some cases). The question becomes "how much of the Cython behaviour should ArrayType try to replicate and I'm not really sure? I suspect it might be better not doing any index checking and just passing it through to the list.
Either way I'm not sure what you should do about it in your patch :s

Either way I'm not sure what you should do about it in your patch :s
Well, it seems to me like it lies out of the scope of my changes to cython.declare and cython.cast. Personally, I would prefer the slicing matches the Cython behavior. Also, we can add isinstance(ix, numbers.Integral), before the check

Well, it seems to me like it lies out of the scope of my changes to cython.declare and cython.cast. Personally, I would prefer the slicing matches the Cython behavior. Also, we can add isinstance(ix, numbers.Integral), before the check

Yes I agree it's out of scope for that. Thanks for the fixing the init. I'm sure I can work out something to get the tests to work

I think part of the issue is that I've ended up trying to use two mechanisms

EvalWithTempExprNode and ResultRefNode to initially move the iterator out of the generator expression.
Creating semi-hidden internal variables with names .0, .1 etc., because it the EvalWithTempExprNode wasn't capable of crossing scope boundaries. (As an aside: I think the semi-hidden nature is OK, since CPython creates similarly named variables that you can see by calling locals() within a generator expression.

I think the solution is to ditch the first method (since it didn't actually do what I wanted) and just do the second method from the start. This hopefully cuts down on a good chunk of the special infrastructure. I'd recommend not reviewing this further until I've tried that, since it'll likely bring large changes.

Some of the special-casing (i.e. if isinstance(something, ResultRefNode)) seems unavoidable to me, unfortunately, although it'll likely look a bit different after the changes - for the moment I really don't see how to wrap things in an "evaluated elsewhere" reference and have that reference be 100% transparent (except where it needs to be opaque)...

I've pushed a small change, really just moving the new infrastructure round a bit so it's not mixed in with the optimization code.
I had a go at the changes described in https://github.com/cython/cython/pull/3323#issuecomment-609038043 - it didn't seem to cut down on the amount of infrastructure but was introducing a whole new set of problems that I was having to special-case. So that's been put on hold for the moment.
I'll think about it some more, but at the moment I'm struggling to see how to cut this down much.

Needs a rebase/merge to clear conflicting files.

Needs a rebase/merge to clear conflicting files.

Hopefully superseded but I'll close it properly once the newer version is fully updated. Marking as draft for now just to stop anyone deciding to merge it (not that I think that's likely to happen)

Superceded by #4254


Don't add codechecker tasks if not enabled

It doesn't make sense to add the tasks if codechecker is not enabled.
This also has the advantage that it gives an error if somebody
tries to execute a codechecker task but codechecker is no enabled.
Signed-off-by: Pascal Bach<EMAIL_ADDRESS>

Looks good. I will merge if theses build are successfull: https://github.com/samdolt/meta-codechecker/actions/runs/1257003233


graphql: resolve __typename if not exists in the resolved object

Fixes https://github.com/graphprotocol/graph-node/issues/3114
Given the following query:
{
  __typename # this one is the problematic one
  purposes {
    __typename
  }
}

Should return:
{
   "__typename": "Query",
   "purposes": [
      {
         "__typename": "Purpose"
      }
   ]
}

But it fails to resolve "__typename": "Query",, because we don't handle __typename not only for root types but all other types.
It works for Purpose for example because __typename is added by resolvers level(prefetch) for other object types.

Cool! thanks @ardatan
At the moment we don't have mutations, and querying for the root Subscriptions __typename is not allowed in the spec (enforced via validations). So this fix should cover all relevant use-cases.

@ardatan should we add a test for this?

@Urigo I added a test to check if __typename is correctly resolved for Query


React Native Maps for IOS Errors and Build Failed

Hi i tried following the link: [https://github.com/anhtuank7c/maps-example], but still no luck. It still shows:

Then when i try to build the project in xcode, it return 1 error:

/Users/gozali/Library/Developer/Xcode/DerivedData/mylaritta-cfgdrrdltwgfgteordinfggkrlvi/Build/Products/Debug-iphonesimulator/react-native-maps/libreact-native-maps.a(SMCalloutView.o)
ld: 298 duplicate symbols for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

i also tried react-native link, and it can't seem to link maps to IOS, it only refers to android. can someone please help?
Here's my app delegate:
/**
 * Copyright (c) 2015-present, Facebook, Inc.
 * All rights reserved.
 *
 * This source code is licensed under the BSD-style license found in the
 * LICENSE file in the root directory of this source tree. An additional grant
 * of patent rights can be found in the PATENTS file in the same directory.
 */

#import "AppDelegate.h"

#import <React/RCTRootView.h>

#import <GoogleMaps/GoogleMaps.h>

@implementation AppDelegate

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
  NSURL *jsCodeLocation;

  [GMSServices provideAPIKey:@"AIzaSyAQb_YvUdM6pF47TJto1DZ6IaOzaGOKerI"];

  /**
   * Loading JavaScript code - uncomment the one you want.
   *
   * OPTION 1
   * Load from development server. Start the server from the repository root:
   *
   * $ npm start
   *
   * To run on device, change `localhost` to the IP address of your computer
   * (you can get this by typing `ifconfig` into the terminal and selecting the
   * `inet` value under `en0:`) and make sure your computer and iOS device are
   * on the same Wi-Fi network.
   */

  jsCodeLocation = [NSURL URLWithString:@"http://localhost:8081/example/index.bundle?platform=ios&dev=true"];

  /**
   * OPTION 2
   * Load from pre-bundled file on disk. The static bundle is automatically
   * generated by "Bundle React Native code and images" build step.
   */

  //   jsCodeLocation = [[NSBundle mainBundle] URLForResource:@"main" withExtension:@"jsbundle"];

  RCTRootView *rootView = [[RCTRootView alloc] initWithBundleURL:jsCodeLocation
                                                      moduleName:@"AirMapsExplorer"
                                               initialProperties:nil
                                                   launchOptions:launchOptions];

  self.window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
  UIViewController *rootViewController = [UIViewController new];
  rootViewController.view = rootView;
  self.window.rootViewController = rootViewController;
  [self.window makeKeyAndVisible];
  return YES;
}

@end

I recently made a fresh example build with pods and GoogleMaps with no issues.
https://github.com/alvelig/react-native-maps-pods-example.
Try to check it out or build through the README.MD. If it does not help, let me know.


Release v1.1.8

lint & format
lint
chore(style): set up prettier
doc(README): move synopsis section into table (#38)

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

1140 of 1219 (93.52%)  changed or added relevant lines in 33 files are covered.
5 unchanged lines in 4 files lost coverage.
Overall coverage decreased (-0.6%) to 91.386%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

rr/aaaa.js
34
35
97.14%

rr/ipseckey.js
52
53
98.11%

rr/caa.js
39
41
95.12%

rr/dnskey.js
46
48
95.83%

rr/soa.js
54
57
94.74%

rr/txt.js
37
40
92.5%

rr/openpgpkey.js
9
13
69.23%

rr/nsec3.js
53
63
84.13%

rr/sig.js
14
24
58.33%

rr.js
98
111
88.29%

Files with Coverage Reduction
New Missed Lines
%

rr/rrsig.js
1
44.44%

rr/sig.js
1
44.34%

rr/soa.js
1
93.37%

rr.js
2
88.75%

Totals

Change from base Build<PHONE_NUMBER>:
-0.6%

Covered Lines:
3457

Relevant Lines:
3711

💛  - Coveralls


Update dotenv to 0.13 (or allow it when it was a range already)

CI says that there is coming a 0.10.1 dotenv version from somewhere that fails to build.

Yeah, it's coming from infer_schema_macros 1.4.0 which is used in diesel_infer_schema, no idea why it makes compilation fail though...

With the state that dotenv is in I'd rather see it get removed or rewritten and replaced. Hope the original creator is doing okay.

ping @Eijebong, any updates?

Already fixed by #2528

Already fixed by #2528


Sort metrics in multiple requests.

Metrics in multiple requests returns in random order.

Needed sort by metric name if you request only one host
And if you request several hosts, sort by hostname and metric name

Fixed by 8eb9c582f31229efea0e8dc90a9c25cb5b23de89

sort by hostname not working, tested in version 3.9.1

@RandomReaper could you post a screenshot of your query?

Here you go:

Host is "/.*/" and order is the order in the hosts have been added to zabbix.

I think it's due to sorting by name before setAliasByRegex(). I should check.

without setting alias, the order is still wrong : 030 is still after 037, still the order in which the machines were added into zabbix.

@RandomReaper Could you please open a new issue for this bug?


feature suggestion: api keys

Prerequisites

[x] I am running the latest version. (up upgrade)
[x] I searched to see if the issue already exists.
[x] I inspected the verbose debug output with the -v, --verbose flag.
[x] Are you an Up Pro subscriber?

not an issue, but a suggestion, what about api keys that you can set via env or up.json

woops just saw this https://github.com/apex/up/issues/625

Where?🤔

Perhaps referring to https://github.com/apex/up/issues/105#issuecomment-380694848


Initial commit copying in Drake-specific style rules...

... and a first draft of a maintainer process.

This change is

For general attention of the platform review team:
+@sherm1 +@jwnimmer-tri +@sammy-tri +@SeanCurtis-TRI +@soonho-tri +@EricCousineau-TRI +@russtedrake +@psiorx
This is a proposal to move our C++ style guide out of the drake docs and into a fork of the google styleguide (which the docs will link to via a github-pages URL).  This will both give us a single page with all C++ style rules together and base our rules on a static version of the google rules which we can update from upstream when and how we prefer.
If adopted, a second (and possibly third) PR will follow to update Drake docs to point to the github-pages page for this cppguide.html.
A rendered view of the html can be viewed here: https://rawgit.com/ggould-tri/styleguide/start-drake-styleguide/cppguide.html .
This PR is intended as a straight copy of the existing styleguide rules.  If you believe I have omitted, changed, or substantially re-shaded any of our style rules, please defect that.

Review status: 0 of 3 files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Reviewed 2 of 3 files at r1.
Review status: 2 of 3 files reviewed at latest revision, 9 unresolved discussions.

cppguide.html, line 12 at r1 (raw file):

Drake C++ Style Guide

FYI This is fine (and we do attribute google/styleguide below), but if there was a way to clearly credit the Google C++ styleguide here at the very first thing you see on the page, I think that would be both kind, and more immediately clarify to the reader what's going on here).

cppguide.html, line 491 at r1 (raw file):

You should include all the headers that define the symbols you rely
upon, except in the unusual case of forward
declaration. If you rely on symbols from bar.h,

FYI Consider after the period adding a citation to the Exception below, for clarity.

cppguide.html, line 1033 at r1 (raw file):

keyword for conversion operators and single-argument
constructors.

FYI To me, this reads like something that belongs at the end of this section, rather than the start.

cppguide.html, line 1993 at r1 (raw file):

Drake Magic

FYI Consider leaving this Google section relatively more intact, and instead inserting the Drake Magic section after it.  The CSS makes it reasonably clear, but saying that smart pointers are "Drake Magic" was a bit confusing at first.

cppguide.html, line 2265 at r1 (raw file):

Rules for Main

This seems wrong.

cppguide.html, line 3049 at r1 (raw file):

ptrdiff_t is allowed when doing arithmetic on bare pointers (this is very rare). Do not use it as a generic “large signed integer” type, nor as a generic “index into a matrix” type.

Unisgned types are prohibited except in the specific cases of:

FYI Should these be in the "On Unsigned Integers" section below?

cppguide.html, line 4364 at r1 (raw file):

  Note that the Drake multibody tree and related types have their own naming
  standard, "Monogram Notation", described
  in drake/multibody/multibody_doxygen.h, that applies to all

This should be a hyperlink to the Doxygen.

cppguide.html, line 5988 at r1 (raw file):

that are declared const need not be initialized in this manner
since the compiler requires that they be initialized in the
constructor's initializer list.

FYI This is a bit of scary wall-of-text in rendered form.  Is there a way to add some whitespace or formatting to lighten it up?

cppguide.html, line 6052 at r1 (raw file):

Windows Code

FYI Consider leaving this intact (with nondrake CSS), with a
Redacted, for clarity vs upstream?

Comments from Reviewable

 This will be awesome, thanks!
I was considering that we should have someone test the README.md before merge, but on second thought, testing it the first time we need it (in a month or three) and patching it up then is likely a better use of time.

Reviewed 1 of 3 files at r1.
Review status: all files reviewed at latest revision, 11 unresolved discussions.

README.md, line 66 at r1 (raw file):

 * Add a "drake" remote for the Drake styleguide and make it the default
   upstream.  Note that for compatibility with Google, we use the branch
   ```gh-pages``` as our master:

FYI Affirm for me that you have gh-pages set as a "protected" (no push) branch in the github admin for this repo?

README.md, line 81 at r1 (raw file):

    git co gh-pages
    git pull

FYI I think we want --ff-only here?  Our gh-pages vs drake/gh-pages should never have merge commits.

Comments from Reviewable

, thank you indeed!

Review status: all files reviewed at latest revision, 20 unresolved discussions.

cppguide.html, line 2 at r1 (raw file):

<!DOCTYPE html>

DOM/CSS-related nit: Should nodrake and drake actually be classes (which I believe are not meant to be unique) rather than ids (which I believe are meant to be unique)
Searching around: CSS Tricks article?

cppguide.html, line 19 at r1 (raw file):

Background

Forked and adapted from the Google style guide.  Many references

BTW Can there be a hyperlink to the original style guide here?
(And could  a quick mention of the README possibly be made, to increase traceability?)

cppguide.html, line 40 at r1 (raw file):

Most code in Drake conforms to the requirements in this guide.

BTW Could this all be placed within , with appropriate boundaries for showing style difference?

cppguide.html, line 2018 at r1 (raw file):

    Doxygen.

If you decide to use Doxygen formatting hints, then those must render
    correctly. For instructions on how to generate the Doxygen website,

BTW minor nit Could "Doxygen website" be specified as "Drake Doxygen website"?

cppguide.html, line 2036 at r1 (raw file):

Allocation Rules

BTW nit consistency Should this be "Memory Allocation", given that "Rules" is implicit in other sections?

cppguide.html, line 2211 at r1 (raw file):

Eigen-Specific Rules

BTW nit consistency Same as above, including "Rules" in the title seems a tad redundant.

cppguide.html, line 2234 at r1 (raw file):

Rules for Assertions

Never use assert() from <cassert>.

Missing
 element (indentation looks funky when rendered).

cppguide.html, line 2485 at r1 (raw file):

On their face, the benefits of using exceptions

Should this entire
 be made nodrake, since it is providing the rationale for a decision that we do not use?

cppguide.html, line 3049 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI Should these be in the "On Unsigned Integers" section below?

Also, small typo: "s/Unisgned/Unsigned"

cppguide.html, line 6334 at r1 (raw file):
Is there a mention of "test files" naming?
Specifically, from our Sphinx:

The test file for the library declarations in drake/foo/bar.h should be drake/foo/test/bar_test.cc. (#2182)

Comments from Reviewable

Review status: all files reviewed at latest revision, 20 unresolved discussions.

cppguide.html, line 3049 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
Also, small typo: "s/Unisgned/Unsigned"

Please don't mix two defects in the same discussion, it messes with the workflow.

Comments from Reviewable

I cannot express how much I like having this all in one place. Grazi!

Review status: all files reviewed at latest revision, 25 unresolved discussions.

cppguide.html, line 12 at r1 (raw file):
FYI

Google C++ Style Guide for Drake

cppguide.html, line 43 at r1 (raw file):
FYI On my screen, the strikethrough text is almost completely unreadable. I vote for a slightly different style.  Also, still isn't clear when the strikethrough would be used -- we've got mountains of text that is from Google convention and (by definition) relevant, that isn't stricken. So, what is the degree of "relevancy" that does get struck through?
From subsequent reading, I believe I've inferred the rule. Perhaps:

Where Drake-specific rules contradict previous Google conventions, the text is retained for reference but otherwise marked with a strikethrough. The style is intentionally difficult to read in the normal course of perusal. Copying-and-pasting, or even simply highlighting the text, will facilitate reading if necessary.

cppguide.html, line 2518 at r1 (raw file):

std::nested_exception.

There is an exception to

FYI Does this enclose enough of the text in strikethrough?  The discussion is still valid, but the conclusions have limited application to Drake.

cppguide.html, line 4386 at r1 (raw file):

my_useful_class.cc

FYI In my browser, I didn't get underscores.

cppguide.html, line 4390 at r1 (raw file):

myusefulclass.cc

myusefulclass_test.cc // _unittest and _regtest are deprecated.

test/myusefulclass_test.cc (For tests for library-public methods of MyUsefulClass; see #2182.)

FYI We don't consider all of these names to be valid for drake, do we? We seem to be strictly of the _ camp. Worth elevating that to a rule rather than just a habit?

cppguide.html, line 5805 at r1 (raw file):

not the variable name. In other words use "const MyClass&
foo;"; instead of "const MyClass &foo;". This is what
is enforced

BTW These examples would be more readable in a code block with trailing comments along the lines of // Bad - & in wrong place, etc. (Particularly, as they would be two lines right on top of each other where the position of the & would be apparent.

Comments from Reviewable

Review status: all files reviewed at latest revision, 24 unresolved discussions.

cppguide.html, line 3049 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Please don't mix two defects in the same discussion, it messes with the workflow.

(Good to note...I've been guilty of this sin...)

Comments from Reviewable

Review status: all files reviewed at latest revision, 25 unresolved discussions.

cppguide.html, line 12 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
FYI

Google C++ Style Guide for Drake

Thanks.

cppguide.html, line 3049 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
(Good to note...I've been guilty of this sin...)

Thanks.

Comments from Reviewable

Awesome, @ggould-tri! Thanks!

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 28 unresolved discussions.

cppguide.html, line 2283 at r1 (raw file):

  or more readable also implies more correct).

Minimize template requirements on public interfaces.

Avoid explicit template instantiations in cc files when possible.

BTW is this actually a Drake rule? We seem to be moving the opposite direction with explicit instantiations being the goal.

cppguide.html, line 3060 at r1 (raw file):

          risk acceptable in trade for allowing this common, clutter-reducing
          practice. For discussion, see
          stackoverflow

BTW this extra indent level doesn't seem helpful. Consider just treating the third point as a separate bullet, even though it is a size_t refinement.

include/styleguide.css, line 272 at r1 (raw file):

  text-decoration: line-through;
  font-size: 90%;
  color: pink;

BTW maybe too hard to read? Consider full size and/or a darker color.

Comments from Reviewable

cppguide.html, line 3050 at r1 (raw file):

ptrdiff_t is allowed when doing arithmetic on bare pointers (this is very rare). Do not use it as a generic “large signed integer” type, nor as a generic “index into a matrix” type.

Unisgned types are prohibited except in the specific cases of:

Small typo: "s/Unisgned/Unsigned"
(splitting off into separate comment - sorry about that!)

Comments from Reviewable

Review status: 0 of 3 files reviewed at latest revision, 29 unresolved discussions.

cppguide.html, line 2 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
DOM/CSS-related nit: Should nodrake and drake actually be classes (which I believe are not meant to be unique) rather than ids (which I believe are meant to be unique)
Searching around: CSS Tricks article?

OK My CSS consultant agrees with you.

cppguide.html, line 12 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Thanks.

OK Sounds good to me.

cppguide.html, line 19 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
BTW Can there be a hyperlink to the original style guide here?
(And could  a quick mention of the README possibly be made, to increase traceability?)

OK Excellent suggestion!

cppguide.html, line 40 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
BTW Could this all be placed within , with appropriate boundaries for showing style difference?

I tried this and found it unclear and ugly.

cppguide.html, line 43 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
FYI On my screen, the strikethrough text is almost completely unreadable. I vote for a slightly different style.  Also, still isn't clear when the strikethrough would be used -- we've got mountains of text that is from Google convention and (by definition) relevant, that isn't stricken. So, what is the degree of "relevancy" that does get struck through?
From subsequent reading, I believe I've inferred the rule. Perhaps:

Where Drake-specific rules contradict previous Google conventions, the text is retained for reference but otherwise marked with a strikethrough. The style is intentionally difficult to read in the normal course of perusal. Copying-and-pasting, or even simply highlighting the text, will facilitate reading if necessary.

OK That is a better phrasing.  And yes, the unreadability was deliberate, although it may have been too extreme -- I'm not really stylistically inclined.  Per sherm's comment on the CSS I will try some alternatives, but asking my to judge style is like asking a mole what color to paint your roof.  I encourage you to play with the CSS and find something you like better!

cppguide.html, line 491 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI Consider after the period adding a citation to the Exception below, for clarity.

Done.

cppguide.html, line 1033 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI To me, this reads like something that belongs at the end of this section, rather than the start.

Done.

cppguide.html, line 1993 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI Consider leaving this Google section relatively more intact, and instead inserting the Drake Magic section after it.  The CSS makes it reasonably clear, but saying that smart pointers are "Drake Magic" was a bit confusing at first.

The "Google Magic" section was misnamed to begin with; little of it had anything to do with google, it's mostly just miscellany.  So I'm renaming it "Miscellany".
Does that look better to you?

cppguide.html, line 2018 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
BTW minor nit Could "Doxygen website" be specified as "Drake Doxygen website"?

Done.

cppguide.html, line 2036 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
BTW nit consistency Should this be "Memory Allocation", given that "Rules" is implicit in other sections?

Done.

cppguide.html, line 2211 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
BTW nit consistency Same as above, including "Rules" in the title seems a tad redundant.

Done.

cppguide.html, line 2234 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
Missing
 element (indentation looks funky when rendered).

Done.

cppguide.html, line 2265 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
This seems wrong.

Done.

cppguide.html, line 2283 at r1 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
BTW is this actually a Drake rule? We seem to be moving the opposite direction with explicit instantiations being the goal.

This is a rule in our Drake styleguide; I'm trying to just transcribe with this change.  If we are changing our approach, I'd rather PR that separately.

cppguide.html, line 2485 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
Should this entire
 be made nodrake, since it is providing the rationale for a decision that we do not use?

It still provides the rationale for the no-catch rule, so I think it is still relevant.
However as it has confused both you and Sean I will try to add a clarifying rewording.

cppguide.html, line 2518 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
FYI Does this enclose enough of the text in strikethrough?  The discussion is still valid, but the conclusions have limited application to Drake.

See above.

cppguide.html, line 3049 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Thanks.

Done

cppguide.html, line 3050 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
Small typo: "s/Unisgned/Unsigned"
(splitting off into separate comment - sorry about that!)

Done.

cppguide.html, line 3060 at r1 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
BTW this extra indent level doesn't seem helpful. Consider just treating the third point as a separate bullet, even though it is a size_t refinement.

Done.

cppguide.html, line 4364 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
This should be a hyperlink to the Doxygen.

Done.

cppguide.html, line 4386 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
FYI In my browser, I didn't get underscores.

This issue is present in the original guide as well.
FYI We don't consider all of these names to be valid for drake, do we? We seem to be strictly of the _ camp. Worth elevating that to a rule rather than just a habit?

This PR is just merging of existing text.  I expect we will have a few discovered rules as a result of this merger, but I would prefer to PR and debate those separately.

cppguide.html, line 5805 at r1 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
BTW These examples would be more readable in a code block with trailing comments along the lines of // Bad - & in wrong place, etc. (Particularly, as they would be two lines right on top of each other where the position of the & would be apparent.

Done.

cppguide.html, line 5988 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI This is a bit of scary wall-of-text in rendered form.  Is there a way to add some whitespace or formatting to lighten it up?

Done.

cppguide.html, line 6052 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI Consider leaving this intact (with nondrake CSS), with a
Redacted, for clarity vs upstream?

Done.

cppguide.html, line 6334 at r1 (raw file):
Previously, EricCousineau-TRI wrote…
Is there a mention of "test files" naming?
Specifically, from our Sphinx:

The test file for the library declarations in drake/foo/bar.h should be drake/foo/test/bar_test.cc. (#2182)

Yes, I incorporated this with slight rewording into the "File Names" section, circa line 4404.

README.md, line 66 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI Affirm for me that you have gh-pages set as a "protected" (no push) branch in the github admin for this repo?

Good catch.

README.md, line 81 at r1 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI I think we want --ff-only here?  Our gh-pages vs drake/gh-pages should never have merge commits.

Sounds right to me, at least until we have done some nontrivial examples.

include/styleguide.css, line 272 at r1 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
BTW maybe too hard to read? Consider full size and/or a darker color.

Done.

Comments from Reviewable

Awesomely Awesome!
I really wish that Google wrote the guide in a markdown (or any more readable format) so that we do not need to edit HTML directly. But I guess there is nothing we can do about it for now.

Reviewed 2 of 3 files at r1.
Review status: 0 of 3 files reviewed at latest revision, 15 unresolved discussions.

cppguide.html, line 1152 at r1 (raw file):

A class must declare its own copy and move operations, and must
not silently inherit the disposition of a base class.  Prefer to
use drake/common/drake_copyable.h macros (either

FYI, I think it helps readers if we provide a hyperlink to https://github.com/RobotLocomotion/drake/blob/master/drake/common/drake_copyable.h
drake/common/drake_copyable.h

README.md, line 26 at r1 (raw file):

When making a change, annotate an html tag surrounding the new material with
```id="drake"```.  This makes it easy for readers to see Drake-relevant

FYI, META, for inlining code, a single backtick suffices.
`id="drake"`. This makes it ...

Reference: https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#inline

README.md, line 62 at r1 (raw file):

            git clone URL_YOU_JUST_COPIED
            cd styleguide

I found that these lines are not rendered as expected. Please consider the following:
   * Clone it on your local machine:
       ```bash
       git clone URL_YOU_JUST_COPIED
       cd styleguide
       ```

---

*[README.md, line 96 at r1](https://reviewable.io:443/reviews/robotlocomotion/styleguide/1#-Kv7lQrpuVgOzBlaCHzl:-Kv7lQrpuVgOzBlaCHzm:b-c2zqvh) ([raw file](https://github.com/robotlocomotion/styleguide/blob/f68eb16deb75ff6df998ae33f760c0c9faabf0e0/README.md#L96)):*
> ```Markdown
> ```RobotLocomotion/styleguide```, not ```google/styleguide```.
>
> You should add all of the platform reviewers to the resulting PR.
> ```

Maybe it's useful to have a link to the list of platform reviewers? Here is a text snippet if you like the idea:

You should add all of the platform reviewers to the resulting PR.

---

*Comments from [Reviewable](https://reviewable.io:443/reviews/robotlocomotion/styleguide/1#-:-Kv7qTtnBS19LjP-iY81:b-qbg8zo)*

Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 13 unresolved discussions.

cppguide.html, line 525 at r2 (raw file):

FYI This is more of an "IWYU exception", not anything related to forward declarations.

cppguide.html, line 3157 at r2 (raw file):

type.

Unsigned types are prohibited except in the specific cases of:

The bullets below should be purple, but are not.

Comments from Reviewable

Review status: all files reviewed at latest revision, 14 unresolved discussions.

cppguide.html, line 49 at r2 (raw file):

text, will facilitate reading if necessary.

Rules specific to Drake are highlighted in purple for

I just had an epiphany. Rather than explicitly describing the style, it should be generic so that the style can change without this text. E.g.,
Rules specific to Drake are highlighted like this.

Comments from Reviewable

Review status: 1 of 3 files reviewed at latest revision, 14 unresolved discussions.

cppguide.html, line 1152 at r1 (raw file):
Previously, soonho-tri (Soonho Kong) wrote…
FYI, I think it helps readers if we provide a hyperlink to https://github.com/RobotLocomotion/drake/blob/master/drake/common/drake_copyable.h
drake/common/drake_copyable.h

Done.

cppguide.html, line 49 at r2 (raw file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
I just had an epiphany. Rather than explicitly describing the style, it should be generic so that the style can change without this text. E.g.,
Rules specific to Drake are highlighted like this.

Nice!

cppguide.html, line 525 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
FYI This is more of an "IWYU exception", not anything related to forward declarations.

Done.

cppguide.html, line 3157 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
The bullets below should be purple, but are not.

Done.  Good catch!

README.md, line 26 at r1 (raw file):
Previously, soonho-tri (Soonho Kong) wrote…
FYI, META, for inlining code, a single backtick suffices.
`id="drake"`. This makes it ...

Reference: https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#inline

Done. Also id was wrong.

README.md, line 62 at r1 (raw file):
Previously, soonho-tri (Soonho Kong) wrote…
I found that these lines are not rendered as expected. Please consider the following:
   * Clone it on your local machine:
       ```bash
       git clone URL_YOU_JUST_COPIED
       cd styleguide
       ```

It appears correct to me at https://github.com/ggould-tri/styleguide/tree/start-drake-styleguide

README.md, line 96 at r1 (raw file):
Previously, soonho-tri (Soonho Kong) wrote…
Maybe it's useful to have a link to the list of platform reviewers? Here is a text snippet if you like the idea:
You should add [all of the platform reviewers](http://drake.mit.edu/developers.html#review-process) to the resulting PR.

Done.

Comments from Reviewable

Reviewed 2 of 2 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 10 unresolved discussions.

Comments from Reviewable

Reviewed 1 of 3 files at r2, 1 of 2 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 9 unresolved discussions.

Comments from Reviewable

Pinging @ericcousineau-tri on open discussions.

Review status: all files reviewed at latest revision, 6 unresolved discussions.

Comments from Reviewable

Reviewed 1 of 3 files at r2, 1 of 2 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable

Reviewed 1 of 3 files at r2, 1 of 2 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Dismissing reviewers who have not commented.
-@RussTedrake  -@psiorx
They will have a further chance to object on the Drake commit.

Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable


Reworked to use maven resolver

I've reworked the project to use maven-resolver as opposed to the depreciated sonatype aether.
One important change is that DefaultArtifact has been deleted - I've used org.eclipse.aether.artifact.DefaultArtifact as it provides the same functionality.
Testing:
All the original unit tests are passing.

Just tacking on to add that I believe we'll need this update for the Java 21 work


There is no device.js in this folder

so change to /lib/ folder.

@keithcao This is wonderful, thank you! And apologies for the delay.


[WIP] store: add cache-control and etag handling

Essentially this PR extracts the Cache-Control and ETag headers and store them in the remote object. Then if we execute a fetch or run command, this implementation checks whether the image needs to be updated based on the Cache-Control stored info, and if so it fetches a new version from the URL.
TODO:  Test If-None-Match HTTP requests works as expected against a server side that handles ETags.
Fixes  #199

@hectorj2f please run this all through gofmt :-)

@jonboulle Done. I added tests to validate the cache-control requests against testing http servers.

I saw that tests are failing, I am taking a look at it.

@jonboulle With the new version of the fetching operation, I tried to apply simple modifications over the existing code. However, I believe we could reorganize the calls to avoid an extra round trip when doing a GET with "If-None-Match" header. Perhaps, it'd be a good idea to discuss about it via mail or IRC.

@hectorj2f ok, do you want to start a conversation on rocket-dev@?

I'll try to sum up over here:

I see a logic on which we have an entry point function GetImage (called by fetch.go) that decides whether we need to download the image (new remote or remote with an expired image) or we can use a cached image. This goes against the current code on which there is an assumption to downloadACI.

Why did you separate the Store function in remote.go ? I remember it was merged into the Download function. It was not easy to understand the reason behind of it :).

Extra-round trip: Lets imagine a scenario on which you have one image with some cache control information (max_age=1000, ... and ETag=1). After some time > 1000, you type run or fetch commands. Rocket will internally check that the max_age of the image has expired so will make a GET request to the remote asking for the image with the ETag=1 and header If-None-Match. With this configuration, we can have two responses: 1) nothing has changed so no need to touch anything = use the cached image and update cache control; or 2) image has been updated so store new image.

@jonboulle Maybe you can give me some light on these points based on what your expected behavior :+1:

[#319] (https://github.com/coreos/rocket/pull/319)

Sorry this sat around for so long. I think we are going to merge #361 after the next release so this will have to be rebased on top of that.

@philips No problem. I'll rebase it to keep it fresh :+1:

@philips I rebased and simplified the implementation according to #361

@hectorj2f @philips @jonboulle
In the past, and now even more strongly, I'm convinced (as also you stated) that the download code should be factored out from remote and moved inside rkt fetch as it's not really related to the remote store. rkt fetch should take the needed info from the remote (if existing) to determine if the ACI needs to be downloaded, then it should write the ACI and update the remote (calling remote.Store). This will also help avoiding tricky things and code duplication done by this patch to avoid the double roundtrip of etag checking. See #654

@sgotti yes, all good points. So this should be rebased on #654 now, right?

I am not concerned about DB migration with 0.5.0, but probably will start to be in the 0.6.0 timeframe.

@jonboulle Yes! Thanks!

@hectorj2f Would you mind doing the rebase on top of the current state of master? Thank you so much and sorry it has taken so long.

@philips sure, no problem. I'll do so.
On Tue 24 Mar 2015 at 01:40 Brandon Philips<EMAIL_ADDRESS>wrote:

@hectorj2f https://github.com/hectorj2f Would you mind doing the rebase
on top of the current state of master? Thank you so much and sorry it has
taken so long.
—
Reply to this email directly or view it on GitHub
https://github.com/coreos/rocket/pull/319#issuecomment-85262792.

@philips do you want to review this?

@jonboulle I just did a rebase and saw all your changes. I may refactor the code then :)

@hectorj2f urgh, I'm so sorry this is taking such a long time to get in :-(. You are right that it needs a substantial rebase - I just tried to have a go at it but haven't got it unraveled yet. At this stage refactoring might be simpler since the actual download path has changed so much.

@hectorj2f let me know if I can help you refactoring the code and writing the store DB migration code and tests. I'm going to open a PR to bump cznic/ql to pull in some table update fixes and new related features.

@sgotti Yes, sure. I'll update the PR

I'd like to resume this as, without it, updated images published with the same URL won't be downloaded if already in the store causing unexpected behaviors and forcing the user to manually remove the previous image from the store. @hectorj2f sorry to bother you, do you have any update? May I help you?

hey @sgotti. I'll update this PR during this weekend. I believe it won't be hard to add this implementation into the current version. I was quite busy the previous months, DockerCon is coming so...

hi @hectorj2f, do you have some time to rebase the PR?

@iaguis I started reworking a bit the @hectorj2f PR changing some logic, adding db migration and improving the tests. I hope to submit it in the next days. @hectorj2f is it ok for you if I add you as Co-authored-by?

That's great. thanks @sgotti!

@sgotti Go ahead :+1:

@sgotti: You mentioned db migration - I just added a PR for backing up db during migration. Might be relevant to your work - #1177.

This was moved to #1181


Added Producer Consumer Problem code in java

Added Producer Consumer Problem code in java

Closing as this does not seem relevant to the project. PR was closed earlier https://github.com/datahub-project/datahub/pull/6176.


[feature] support alternative container cli in 'flynn docker ...' (podman)

It would be nice if flynn docker ... commands could support different container clis compatible with docker. Specifically, I am recently using podman, which is almost fully compatible to the docker cli but can run entirely in user-namespaces without any daemons etc.
Looking at the subcommand sources, most - if not all - docker commands are implemented with os/exec.Command. If we could simply allow for replacing "docker" with "podman" in all those instances I believe this should be doable?
I am currently trying to find incompatabilities by symlinking podman as docker in my PATH:

[x] login fails with a TLS error, I am not sure yet how to add the CA cert in podman

just add docker.$CLUSTER to [registries.insecure]? https://www.projectatomic.io/blog/2018/05/podman-tls/ --> this does not "magically" work
put the CA certificate in /etc/containers/certs.d/docker.$CLUSTER/*.crt --> success

[ ] podman uses .ContainerConfig instead of .Config in inspect output

this currently blocks further investigation because it requires patching either podman or flynn

This simple patch is enough to make flynn work with podman when podman is aliased as docker in PATH:
diff --git a/cli/docker.go b/cli/docker.go
index c09ee597..bbcae0e1 100644
--- a/cli/docker.go
+++ b/cli/docker.go
@@ -201,7 +201,7 @@ func runDockerPush(args *docopt.Args, client controller.Client) error {
        }

        // get the image config to determine Cmd, Entrypoint and Env
-       cmd := exec.Command("docker", "inspect", "-f", "{{ json .Config }}", image)
+       cmd := exec.Command("docker", "inspect", "-f", "{{ json .ContainerConfig }}", image)
        log.Printf("flynn: getting image config with %q", strings.Join(cmd.Args, " "))
        stdout, err := cmd.StdoutPipe()
        if err != nil {

Of course, this breaks compatability with docker so this is not a "solution". But it appears there are no further incompatabilities between the two clis with regard to flynn.

As discussed in #4466 this is an edge case that can be handled by symlinking podman as docker in your PATH. To trust the Flynn repository, you'll need to add the CA cert in /etc/containers/certs.d/docker.$CLUSTER/*.crt as described above.
The .ContainerConfig vs .Config inconsistency currently requires patching the Flynn CLI for podman, which could break compatability with docker however. See the pull request for details. This seems like a compatability issue with libpod, so closing this issue.


Danger with packaging vscode-jsonrpc twice - Unknown parameter structure

tl;dr Remove dependency on vscode-jsonrpc in example dir because it causes things to break. See this issue https://github.com/microsoft/vscode-languageserver-node/pull/719
Context & The Problem
So after I merged in @CGNonofr 's latest PR into my custom fork, my example all of sudden broke. Specifically, I was getting this trace error:
console-window.ts:11 Couldn't start client Sample Language Client
showMessage @ console-window.ts:11
showMessage @ vscode-api.ts:853
(anonymous) @ client.js:2168
Promise.then (async)
start @ client.js:2164
onConnection @ client.ts:227
webSocket.onopen @ connection.ts:19
connection.js:776 Uncaught (in promise) Error: Unknown parameter structure byName
    at computeSingleParam (connection.js:776)
    at computeMessageParams (connection.js:787)
    at Object.sendRequest (connection.js:929)
    at Object.initialize (connection.ts:107)
    at MonacoLanguageClient.doInitialize (client.js:2220)
    at MonacoLanguageClient.initialize (client.js:2216)
    at client.js:2163
computeSingleParam @ connection.js:776
computeMessageParams @ connection.js:787
sendRequest @ connection.js:929
initialize @ connection.ts:107
doInitialize @ client.js:2220
initialize @ client.js:2216
(anonymous) @ client.js:2163
Promise.then (async)
start @ client.js:2164
onConnection @ client.ts:227
webSocket.onopen @ connection.ts:19

When I debugged into the code, the culprit is that somehow byName kind is not recognized even though the payload looks clean.
parameterStructures: ParameterStructures
kind: "byName"

    function computeSingleParam(parameterStructures, param) {
        switch (parameterStructures) {
            case messages_1.ParameterStructures.auto:
                if (isNamedParam(param)) {
                    return nullToUndefined(param);
                }
                else {
                    return [undefinedToNull(param)];
                }
                break;
            case messages_1.ParameterStructures.byName:
                if (!isNamedParam(param)) {
                    throw new Error(`Recevied parameters by name but param is not an object literal.`);
                }
                return nullToUndefined(param);
            case messages_1.ParameterStructures.byPosition:
                return [undefinedToNull(param)];
            default:
                throw new Error(`Unknown parameter structure ${parameterStructures.toString()}`);
        }
    }

I was extremely confused as Loic's changelog shouldn't impact this at all; in fact, I put a breakpoint at every diff and the new code never got executed. Turns out, the culprit is that ParameterStructures is meant to be a singleton but as  dbaeumer stated in the linked issue, having two packaged instances (one from monaco-languageclient and the other for the example) meant there were two instances. Seems like this is a ticking bomb waiting to happen, and it just so happened that re-compiling the code with the changes somehow exposed this error to occur deterministically for me.
The Solution & Proposal
Remove vscode-jsonrpc in package.json of example and instead import MessageConnection from monaco-languageclient as this seems to be imported in connections.ts and then re-exported in index.ts.
Specifically, my diff looks like below (I can create a PR as well if that's the preference). After my change, it works again :D
-import {MessageConnection} from 'vscode-jsonrpc';
+// import {MessageConnection} from 'vscode-jsonrpc';
 import {
     MonacoLanguageClient,
     CloseAction,
@@ -12,6 +12,7 @@ import {
     MonacoServices,
     createConnection,
     Configurations,
+    MessageConnection,
     WorkspaceConfiguration,
     MonacoProvideCompletionItemsSignature,
 } from 'monaco-languageclient';

Ok, thanks for the bug report!
I already had the exact same issue when debugging it using a local npm dependency. It's a known issue to have duplicated sub-dependencies when using local dependencies so I didn't really mind.
You can create a PR if you feel it, at least you'll get the credit for the fix.
Otherwise just tell me and I'll get a look at it.

Much appreciated @CGNonofr  for the quick response! I created the PR but honestly don't mind if you need to end up doing it due to permission issues.
https://github.com/TypeFox/monaco-languageclient/pull/255

I already had the exact same issue when debugging it using a local npm dependency. It's a known issue to have duplicated sub-dependencies when using local dependencies so I didn't really mind.

How did you end up resolving it? I'm actually having some issues taking my example and porting it over to my company's application because of this issue. I do see that I have two instances of vscode-jsonrpc though, so I'm hoping the fix is similar.

I just resolved the issue by not using local dependencies at all
To test it I just build it, package it, then install the package.
If you need local dependencies and you use webpack, there is a solution using aliases with an absolute path.

Thanks for the very detailed explanation and the PR. The PR is merged now.

I met this problem because there's 2 vscode-languageserver-protocol package. One in node_modules/vscode-languageserver-protocol, one in another-package/node_modules/vscode-languageserver-protocol.
I believe the protocol is exposed in the API hence should be declared as peerDependencies, not dependencies.


feat(string_extension): add matchesWith method

Coverage increased (+1.03%) to 87.395% when pulling b0ad0fec3d092bfeaabb1d5b86ea8723cc263652 on feat/add-string-extension-matches-with into a0e6cde6f3bd7efa562b3ff5b43ee504f6b2e366 on main.


New command: spo listitem attachment remove

Usage
m365 spo listitem attachment remove [options]
Description
Removes an attachment from a list item
Options

Option
Description

-u, --webUrl <webUrl>
URL of the site where the listitem is located.

--listId [listId]
ID of the list. Specify either listTitle, listId or listUrl.

--listTitle [listTitle]
Title of the list. Specify either listTitle, listId or listUrl.

--listUrl [listUrl]
Server- or site-relative URL of the list. Specify either listTitle, listId or listUrl.

--listItemId <listItemId>
The ID of the list item.

-n, --fileName <fileName>
Name of the file to remove.

--confirm
Don't prompt for confirmation.

Examples
Remove an attachment from a list item using list tile.
m365 spo listitem attachment remove --webUrl https://contoso.sharepoint.com/sites/project-x --listTitle "Demo List" --listItemId 147 --fileName "File1.jpg"

Remove an attachment from a list item using list URL
m365 spo listitem attachment remove --webUrl https://contoso.sharepoint.com/sites/project-x --listUrl "/sites/project-x/Lists/DemoList" --listItemId 147 --fileName "File1.jpg"

Additional Info
More info regarding the SP REST endpoint to use: https://learn.microsoft.com/sharepoint/dev/sp-add-ins/working-with-folders-and-files-with-rest#working-with-files-attached-to-list-items-by-using-rest

Made few tiny changes, but looks good!

Can I work on this?

All yours!


Wrong behavior in mergeSeries

Description
I have a CT series with 67 frames.
First frame in series

is in axial projection and shows how slices was made by machine (each ~horizontal line on this picture equal to one frame in series).
All other frames are in coronal projection.
Size of info frame is smaller then all other frames, so: packTo8Bits() crushes with:

TypeError: Cannot read property 'pixelData' of undefined

Files
broken series - info frame.zip
broken series - normal frame.zip
Expected Results
These frames shouldn't be merged into one stack, because their frame.rows and frame.columns are different. DICOM tag (0008,0008) ImageType is also different.
Versions

AMI: v0.0.23dev

Moreover, if info frame is opened in AMI all overlay info is missing


Filter Iris warnings?

Would it be possible to filter Iris warning using warnings?
https://docs.python.org/3.1/library/warnings.html
They make the log on the stdout pretty verbose.

After switching to iris 2 #451, most of the warnings should disappear. The ones that don't should probably be addressed instead of filtered.


Severe Weather app

Please updated font to tom-thumb, the selected font is hard to read, I loaded it locally, modified and pisplayed in pixlet serve, tom-thumb works good, and line not on top of each other.
    ## Main Alert Text
    mainAlertText = alert["properties"]["event"]

    mainAlertTextWrappedWidget = render.WrappedText(
        content = mainAlertText.upper(),
        align = "center",
        font = "tom-thumb",  # tiny
        color = "#FF0000",  # red
    )

See attacked zip webp from export on pixlet serve
tidbyt-preview-1712115255238.webp.zip

Cummunity app name:  severewxalertsusa.star

Might also be a solution to use the linespacing attribute in the WrappedText widget to make things a little more readable. Let's tag the app author @rs7q5 to see what he thinks!

What is the original font?
I am not the original author. That would be @aschechter88. I don't think it's super hard to read and you also have to make sure to account for almost all scenarios or longer names.
One possible option though that I don't think the author would care about would be to just add a simple font config of small/large texts (instead of all options). That I think would satisfy all parties.

Original:CG-pixel-3x5-monoPROPOSED#tom-thumbThe spacing concerned mention is moot in irrevelent as in test unfact the tom-thumb actually is more efficient and will display more info so in yesterdays alerts with original: “FOG ADVISERY” on disply showed fog one first line adverisery on second line smoshed together  (Hence hard to read)  format 4x5x4x5x0 the proposed actually fit the full alert on line on (4x6x4x4x1) on the second alerts with was three lines on original font also three lines new font same three lines with one pixel space between the lines so gtheir not smashed together, I can only assume the author didnt set tom-thun oraginanly if it wasnt created yet as effectly they both will effectly only give 4 lines in the pane defined for the message.so to the other comment then sime if you want that original font fine allow font choice tom-thumb or the CG in the options, but realistically the cg3x5 is bad choice.Send from @.*** Apr 4, 2024, at 6:18 AM, rs7q5 @.***> wrote:
What is the original font?
I am not the original author. That would be @aschechter88. I don't think it's super hard to read and you also have to make sure to account for almost all scenarios or longer names.
One possible option though that I don't think the author would care about would be to just add a simple font config of small/large texts (instead of all options). That I think would satisfy all parties.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

Setting linespacing = 1 on the WrappedText widget does fix the problem you are describing with the lines being too close together. Current state:

With linespacing = 1

Switching it to tom-thumb makes W, M, and N nearly indistinguishable from each other so I can understand why the developer may not have picked it.
tom-thumb:

If it were my app, my preference would be to leave the font the same and change the linespacing to 1.

Ok, Let me try it locally setting the linespace, where did you put the linespace tag?  Net however, the code should be corrected, either way the best.  I’ll check how linespacing shows inn HDK later tonight.Send from @.*** Apr 5, 2024, at 5:28 AM, Jake Manske @.***> wrote:
Setting linespacing = 1 on the WrappedText widget does fix the problem you are describing with the lines being too close together. Current state:
Screenshot.2024-04-05.at.5.19.37.AM.png (view on web)
With linespacing = 1
Screenshot.2024-04-05.at.5.18.38.AM.png (view on web)
Screenshot.2024-04-05.at.5.24.06.AM.png (view on web)
Screenshot.2024-04-05.at.5.24.21.AM.png (view on web)
Switching it to tom-thumb makes W, M, and N nearly indistinguishable from each other so I can understand why the developer may not have picked it.
tom-thumb:
Screenshot.2024-04-05.at.5.26.09.AM.png (view on web)
Screenshot.2024-04-05.at.5.26.32.AM.png (view on web)
If it were my app, my preference would be to leave the font the same and change the linespacing to 1.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

It's in the same spot you changed the font in your first post. Instead of changing the font, add the linespacing attribute.

I made a draft PR if you want to take a look. If you're good with it, I can submit it. #2376


[ISSUE-2742] A lighter InsertPlan for single data point insertion

3175

 Quality gate: Failed
242 Issues

 48 Vulnerabilities
 165 Code issues
 29 Anti-patterns

Details

 Quality gate: Failed
111 Issues

 48 Vulnerabilities
 34 Code issues
 29 Anti-patterns

Details


Fallback to nearest available server if user selected is gone

Task/Issue URL: https://app.asana.com/0/488551667048375/1206158080166975/f
Description
Ensure we fallback to nearest server if user selected server is not present
Steps to test this PR
Code review and check unit tests.
Also smoke test NetP.

Current dependencies on/for this PR:

develop

PR #4010   👈

This stack of pull requests is managed by Graphite.


element selector is picking up wrong elements in the latest chrome version.

It picks up the next element of the specified selector.

Please provide reproduction. I'm on the latest versions of everything and having no trouble.

Hi @GitSage , It only happens when we are creating a profile of chrome while disabling the web security.

Full reproduction if you can, please. A step-by-step description of how to make the error including all code.


data: Miraz

Fixes Issue

Changes proposed

Check List (Check all the applicable boxes)

[ ] My code follows the code style of this project.
[ ] My change requires changes to the documentation.
[ ] I have updated the documentation accordingly.
[ ] All new and existing tests passed.
[ ] This PR does not contain plagiarized content.
[ ] The title of my pull request is a short description of the requested changes.

Screenshots

Note to reviewers

@SurajPratap10 @GauravKanwat Thanks! 😄

@SurajPratap10 @GauravKanwat Thanks! 😄

No worries


[Android] Agent.getBuildId() was unable to find a valid build Id

When starting the release build of our app from either Play Store or locally using flutter run --release I get the following error when trying to send a crash report to newrelic:
Agent.getBuildId() was unable to find a valid build Id. Crashes and handled exceptions will not be accepted.
Invalid (null or empty) build ID detected! Crash will be ignored by collector.
ExceptionHelper: java.util.UUID:fromStringJava8(UUID.java:337) RandomUUID[java.lang.IllegalArgumentException] Invalid UUID string:
Unable to get obfuscated flag in crash
Metric.increment() called with value[-1] less then or equal to zero

In settings.gradle I have:
plugins {
    id "dev.flutter.flutter-plugin-loader" version "1.0.0"
    id "com.android.application" version "8.1.0" apply false
    id "org.jetbrains.kotlin.android" version "1.8.22" apply false
    id "com.newrelic.agent.android" version "7.6.0" apply false
}

(also tried 7.5.1)
and in app/build.gradle:
plugins {
    id "com.android.application"
    id "kotlin-android"
    // The Flutter Gradle Plugin must be applied after the Android and Kotlin Gradle plugins.
    id "dev.flutter.flutter-gradle-plugin"
    id "com.newrelic.agent.android"
}

What am I doing wrong?
Environment:
Flutter 3.24.3 • channel stable • https://github.com/flutter/flutter.git
Framework • revision 2663184aa7 (3 weeks ago) • 2024-09-11 16:27:48 -0500
Engine • revision 36335019a8
Tools • Dart 3.5.3 • DevTools 2.37.3

Gradle 8.3

@akaegi have you add proguard rules?  https://docs.newrelic.com/docs/mobile-monitoring/new-relic-mobile-android/install-configure/configure-proguard-or-dexguard-android-apps/

Thank you @ndesai-newrelic, that was indeed the issue. Maybe you can mention in the Flutter docs that these proguard rules need to be set?

@akaegi we are in process to update those documentation for all the hybrid frameworks.


Environment-specific canisters?

Is there a way to declare in dfx.json that a canister should only be deployed to certain environments?
For example, to configure ledger or internet identity canisters for local deployment only.
cc @sesi200 in case this relates to the work mentioned in https://github.com/dfinity/sdk/issues/2199#issuecomment-1125992831

I think you're looking for the remote canister feature: #2010. Does this solve your problem?

Possibly! I'll give it a try and report back. Thanks.

This appears to do what I want, at least locally. I did the following.
I'm not sure yet if it's necessary to specify the candid field again under remote, or if the canister ID is the right one.
{
  "canisters": {
    "internet_identity": {
      "type": "custom",
      "wasm": "result/remote/internet_identity/internet_identity_dev.wasm",
      "candid": "result/remote/internet_identity/internet_identity.did",
      "remote": {
        "candid": "result/remote/internet_identity/internet_identity.did",
        "id": {
          "ic": "rdmx6-jaaaa-aaaaa-aaadq-cai"
        }
      }
    }
  }
}

I'm not sure https://github.com/dfinity/sdk/issues/2085 is what I'd want but perhaps some version of "type": "remote" would improve things.

How important is it to use the correct id for a remote canister. I need to set up some local third-party canisters for development but I don't know their canister IDs. Even for Internet Identity I had to go digging.
I think what I might want is a "only install these locally" option (or something equivalent) without having to provide the extra info.

As I mentioned in https://github.com/internet-identity-labs/NFID-SDK/discussions/141#discussioncomment-3370064, I'll try to use dfx canister install --wasm <path> <canister name> instead.

Apparently that still requires the same entries in dfx.json as before so it doesn't help at all.

How important is it to use the correct id for a remote canister?

Any calls you perform to this canister go to the entered ID. If you enter the wrong ID, the wrong canister IDs will be baked into your wasm if you reference these canisters.

I think what I might want is a "only install these locally" option (or something equivalent) without having to provide the extra info.

Internally, we're discussing about the idea of something like docker-hub for canisters that would make this even more convenient. Unrelated question (i.e. don't take this question as "we're working to wards this"): Would it be a solution for you if the individual projects (say II or the ledger canister) had a ready-to-copy-paste snippet that would contain the relevant remote configuration?

Any calls you perform to this canister go to the entered ID. If you enter the wrong ID, the wrong canister IDs will be baked into your wasm if you reference these canisters.

The thing is, I’m (currently) not referencing those canisters. Both II and NFID provide URLs with domain names for interacting with them.
This might change with some other canisters I plan to add, like canistergeek.

Would it be a solution for you if the individual projects (say II or the ledger canister) had a ready-to-copy-paste snippet that would contain the relevant remote configuration?

It would help if projects like NFID and canister geek provided this info. My main concern is knowing the canister ID.
I also have more general concerns with local vs remote canisters behaving differently, but that’s a different topic.


Introduction/LastPush Task 4/8 doesn't pass a test due to line separator

Introduction/LastPush Task 4/8: Patterns Generator - repeatHorizontally Function, doesn't pass due to line separator.
After implementation of repeatHorizontally function, some test doesn't pass.
The result of my function and expected result are the same.
It seems the problem in expected line separator.
Could you explain please, how to fix or bypass the issue?
My function:
fun repeatHorizontally (pattern: String, n: Int, patternWidth: Int): String {
    val result = StringBuilder()
    pattern.lines().forEach { s: String ->
        val filled = fillPatternRow(s, patternWidth)
        val multiString = filled.repeat(n)
        result.append(multiString).append(newLineSymbol)
    }
    result.removeSuffix(newLineSymbol)
    return result.toString()
}

Feedback and the comparison  result you can find in the attachments.
OS: Windows 10 home edition
IDE: IntelliJ IDEA 2023.2.5 (Ultimate Edition); Build #IU-232.10227.8, built on November 9, 2023; Runtime version: 17.0.9+7-b1000.46 amd64; VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
feedback.txt

I have a similar problem. I can pass Task 4/8 with this function
fun repeatHorizontally(pattern: String, n: Int, patternWidth: Int): String {
    val rows = pattern.lines()
    val sb = StringBuilder()
    for ((index,row) in rows.withIndex()) {
        sb.append(fillPatternRow(row, patternWidth).repeat(n))
        if (index != rows.lastIndex )
            sb.append("\n")    }
    return sb.toString()
}

Note the use of "\n" instead of newLineSymbol = System.lineSeparator() (\r\n in windows)
I've cloned this repository but I still get similar errors (in test.zip attachment) related to tast 8/8
test.zip

@SergeiPavlitz @Vigo61 Please, check now, it should work

Hi.
I've checked again. The situation is the same.(
But I could bypass the problem, in a way which was described by @Vigo61. I've changed the newLineSymbol on "\n".
I reopened the project via the website and clicked the button "Open in Intellij Idea".
Maybe this information can help.
My code:
fun fillPatternRow(patternRow: String, patternWidth: Int): String {
    check(patternRow.length <= patternWidth) { "The row length is bigger than patternWidth" }
    val extendedRow = StringBuilder(patternRow)
    extendedRow.append(separator.toString().repeat(patternWidth - patternRow.length))
    return extendedRow.toString()
}

fun repeatHorizontally(pattern: String, n: Int, patternWidth: Int): String {
    val result = StringBuilder()
    val list = pattern.lines().toMutableList()
    val first = list.removeFirst()
    result.append(fillPatternRow(first, patternWidth).repeat(n))

    list.forEach { s: String ->
        result.append(newLineSymbol)
//        result.append("\n")
        val filled = fillPatternRow(s, patternWidth)
        val multiString = filled.repeat(n)
        result.append(multiString)
    }
    return result.toString()
}

feedback.txt

Hi Sergei,
my version of repeatHorizontally works fine only in task 4 - It's better to use the following function that use newLineSymbol,
taken from the present git
fun repeatHorizontally(pattern: String, n: Int, patternWidth: Int): String {
    val pictureRows = pattern.lines()
    val sb = StringBuilder()
    for (row in pictureRows) {
        val currentRow = fillPatternRow(row, patternWidth)
        sb.append(currentRow.repeat(n))
        sb.append(newLineSymbol)
    }
    return sb.toString()
}

but unluckily in the next tasks I get errors from
fun dropTopFromLine(line: String, width: Int, patternHeight: Int, patternWidth: Int): String {
    val nToDrop = if (patternHeight > 1) {
        patternWidth * width + newLineSymbol.length
    } else {
        0
    }
    val newPattern = line.removeSuffix(newLineSymbol).drop(nToDrop)
    return "$newPattern$newLineSymbol"
} // this function is not mine still from this git

In this case the error is understandable and I'm trying to correct by myself  before opening a new issue, but it need to me a lot of time :-)
The method dropTopFromLine with arguments line=
+---+---+
| o   o |
|   ^   |
|  ---  |
+---+---+
, width=1, patternHeight=5, patternWidth=9 should return
| o   o |
|   ^   |
|  ---  |
+---+---+
But it returns
o   o |
|   ^   |
|  ---  |
+---+---+

@Byte-m-e

I have used return sb.toString().trimIndent() instead of return sb.toString() and this solved the problem for me.

Which task are you talking about, always of 4) or the later?
Many different solution seems to work fine trying to run manually the main function, but i think there is still some error in the test process.
In can pass tasks from 5) to 7) using this function improved with your suggestion (added .trimIndent())
(derived trom repeatHorizzontally, only the first parameter is needed, the others added to pass the test - so this couldn't be the right solution)
fun dropTopFromLine(line: String, width: Int, patternHeight: Int, patternWidth: Int): String {
    if (patternHeight < 2) return line
    val lineRows = line.lines()
    val sb = StringBuilder()
    for (r in lineRows.indices) {
        if (r == 0) continue
        sb.append(lineRows[r])
        sb.append(newLineSymbol)
    }
    return sb.toString().trimIndent()
}

but in task 8) the best function is the original from @nbirillo (from this repository)
fun dropTopFromLine(line: String, width: Int, patternHeight: Int, patternWidth: Int): String {
    val nToDrop = if (patternHeight > 1) {
        patternWidth * width + newLineSymbol.length
    } else {
        0
    }
    val newPattern = line.removeSuffix(newLineSymbol).drop(nToDrop)
    return "$newPattern$newLineSymbol"
}

but not enough good to pass the whole test process (passed 145 of 147 tests)
The following errors happen only during test process - because dropTopFromLine is called only from canvasGenerator and the pattern is passed vie repeatHorizontally
fun canvasGenerator(pattern: String, width: Int, height: Int): String {
    val patternWidth = getPatternWidth(pattern)
    val patternHeight = getPatternHeight(pattern)
    val repeatedPattern = repeatHorizontally(pattern, width, patternWidth)
    val dropped = dropTopFromLine(repeatedPattern, width, patternHeight, patternWidth)
    return "$repeatedPattern${baseGenerator(dropped, dropped, height - 1)}"
}

The method dropTopFromLine with arguments line=
+---+---+
| o   o |
|   ^   |
|  ---  |
+---+---+
, width=1, patternHeight=5, patternWidth=9 should return
| o   o |
|   ^   |
|  ---  |
+---+---+
But it returns
 o   o |
|   ^   |
|  ---  |
+---+---+

<Click to see difference>

org.opentest4j.AssertionFailedError: The method dropTopFromLine with arguments line=
+---+---+
| o   o |
|   ^   |
|  ---  |
+---+---+
, width=1, patternHeight=5, patternWidth=9 should return
| o   o |
|   ^   |
|  ---  |
+---+---+
But it returns
 o   o |
|   ^   |
|  ---  |
+---+---+
 ==> expected: <| o   o |
|   ^   |
|  ---  |
+---+---+> but was: < o   o |
|   ^   |
|  ---  |
+---+---+>

maybe it's time to open a new issue ....

@Byte-m-e Hi
thanks for your suggestions.
I tried the code from this git in a Ubuntu virtual machine. I'm really a dummy user in Linux OS but everything was fine, check passed without errors. I tried the same code in another windows machine and I got the same errors that in my main Windows PC.

@Byte-m-e Hi
thanks for your suggestions.
I tried the code from this git in a Ubuntu virtual machine. I'm really a dummy user in Linux OS but everything was fine, check passed without errors. I tried the same code in another windows machine and I got the same errors that in my main Windows PC.
I tried your code snippets and your suggestions but i can't pass the check of Task 8 under Windows 10
Under that OS I get the minimum number of errors (2 out of 147) using without modification the code from this git repository.
I believe that whoever wrote the test code should modify it so that it does not give different results depending on the operating system.
When I have time and inclination maybe I will do some more tests before opening a new ticket
But at the moment I think I have wasted enough time on this problem although perhaps it could have been a useful exercise.
Thanks again

@Byte-m-e @Vigo61 please check again

@nbirillo I tested only the code from this git in lastpush 4 and 8 and it seems to work fine in my windows PC now! 👍
I tested also some of my old code: My code fails but because it's wrong


Implement and test the object pose estimation EKF

Time estimate: 20 hours
Deadline:  07.02.22
Description of task:
Find and squash the bugs that give the invalid results from the EKF node, and finish implementing the vision-pose estimation filter. As the problem is very likely related to frames being used in the simulator and/or one of the conformed transformation matrices, it is important to test the node using the rosbags taken at the MC-lab.

Issue 294 - reference to the original object EKF issue.

Some pointers and specifications for the code:

Use as many ready made tf commands as possible. Make the code as agnostic as possible to frames.
Map out where we cannot use standard tf commands and have to make a rotational object ourselves and minimize the instances of those (only do it when absolutely necessary)
Evaluate the actual need for "transform_to_world" (can we do everything cleanly with tf commands?)
Consider the switch to quaternion parametrization after the ekf works on the bags

The simulation on bag data:
0) Start with only position.

Generate and publish measurements of the gate (transform gt pose topic from qualsys to camera frame).
Feed the measurements to the filter along with drone pose from.
Publish the estimate in QualSys frame again to check if it converges on gt. This is not ideal, but it is better than trying to transform between odom and QualSys. We should just make sure that the middle part of the code works, regardless of what frame comes in and out.

The simulations were performed successfully on sunday  06.02.2022. What we found out and where the problems stemmed from:

Using tf for all transformations always. Don't try to implement rotations in any way yourself.
When one needs a rotation matrix from the quaternion provided by transformation use tf.transformations.quaternion_matrix( q ). This has a lot less ambiguity than euler angles.
The tf lookup function, when called on a parent and child frame gives the rotation from child to parent (we believe)

The EKF node was further rewritten to be quite blind to whatever transformations come into the system and should be more robust for that kind of stuff.

Estimation of the full 6 DOF pose of an object in the world frame is now the focus of the development here. In the period 13-18 february analytic expressions for the measurement model and its jacobian were derived through SymPy and implemented with njit for faster run times. Here is a glimpse into jacobian

hell:

The focus from now is to implement this into ROS and as a measurement model in the ekf measurementmodels.py script.
TODO for 20.02.2022:

[ ] Implement the njit-ed function with ROS (make a self instance on startup of the ekf node and just pass it into ekf  spørsmålstegn x3 )
[ ] Extend the measurement model to include the new states and jacobian
[ ] Test everything with published full gate pose like we did for position and debugg.

This can safely be closed now. The VKF's newest incarnation has all of these issues pretty much irrelevant.