Working 0-1D Chemistry

Description

Adds three new working example cases nD_perfect_reactor, 1D_reactive_shocktube, and 1D_inert_shocktube.
Fixes HLL+Advection, Pressure/Energy calculations, and more (for chemistry).
Removes the horrible misc/viz.py script and instead adds a reusable library in the toolchain for this. See the small, individual, viz.py files for each example case I added.
I put some code I often used to remove case parameters for dimensions above 1 or 2 in one file. Cases can just import mfc.case_utils.
For all of this to work, I created a pyproject.toml for the toolchain so that it can be installed in the venv. You can also pip3 install -e toolchain/.
When ./mfc.sh run passes its internal state to the cases it reads, through CLI arguments, it now does so using the --mfc argument (instead of using positional arguments). This improves the usability of the feature because you no longer have to pass your case a fake first argument when running it using python3.
Now that pyrometheus/pyrometheus#87 has been merged we point directly to pyrometheus's main branch (no longer one of my dev branches).

Type of change

[x] New feature (non-breaking change which adds functionality)

Scope

[x] This PR comprises a set of related changes with a common goal

How Has This Been Tested?
The three example cases recreate simulations from papers referenced in their respective case.py files and READMEs.

Can you add goldenfiles/tests since we now have examples? Even if they are only for 0D or 1D. this would also help ensure the user has installed boost/pyro and other dependencies correctly.

Yes, will do. There is one caveat, however. ./mfc.sh test would build two versions of MFC, the regular, and one using chemistry + h2o2.yml (the reaction mechanism). Are you fine with this? The user need not worry about this happening, they will just MFC "twice". Are you fine with this?

Can you add goldenfiles/tests since we now have examples? Even if they are only for 0D or 1D. this would also help ensure the user has installed boost/pyro and other dependencies correctly.

Yes, will do. There is one caveat, however. ./mfc.sh test would build two versions of MFC, the regular, and one using chemistry + h2o2.yml (the reaction mechanism). Are you fine with this? The user need not worry about this happening, they will just see MFC build "twice". Are you fine with this?

Perhaps... what is the reason? For example, as a user it would be convenient to be able to run a chemistry case, and then switch to a 'normal' case without rebuilding (or vice-versa). Is the mechanism only available at compile time for some reason?

Can you add goldenfiles/tests since we now have examples? Even if they are only for 0D or 1D. this would also help ensure the user has installed boost/pyro and other dependencies correctly.

Yes, will do. There is one caveat, however. ./mfc.sh test would build two versions of MFC, the regular, and one using chemistry + h2o2.yml (the reaction mechanism). Are you fine with this? The user need not worry about this happening, they will just see MFC build "twice". Are you fine with this?

Perhaps... what is the reason? For example, as a user it would be convenient to be able to run a chemistry case, and then switch to a 'normal' case without rebuilding (or vice-versa). Is the mechanism only available at compile time for some reason?

This is transparent to the user. Chemistry is a compile-time feature because it needs to invoke Pyrometheus to generate code for a specific mechanism file. The build "slug" system is in place to prevent rebuilds for the exact reason you described. The slug is (partially) a function of the mechanism. When you switch back to running a "regular" MFC case, it will re-use a previous build.

Can you add goldenfiles/tests since we now have examples? Even if they are only for 0D or 1D. this would also help ensure the user has installed boost/pyro and other dependencies correctly.

Yes, will do. There is one caveat, however. ./mfc.sh test would build two versions of MFC, the regular, and one using chemistry + h2o2.yml (the reaction mechanism). Are you fine with this? The user need not worry about this happening, they will just see MFC build "twice". Are you fine with this?

Perhaps... what is the reason? For example, as a user it would be convenient to be able to run a chemistry case, and then switch to a 'normal' case without rebuilding (or vice-versa). Is the mechanism only available at compile time for some reason?

This is transparent to the user. Chemistry is a compile-time feature because it needs to invoke Pyrometheus to generate code for a specific mechanism file. The build "slug" system is in place to prevent rebuilds for the exact reason you described. The slug is (partially) a function of the mechanism. When you switch back to running a "regular" MFC case, it will re-use a previous build.
    def get_slug(self, case: input.MFCInputFile) -> str:
        if self.isDependency:
            return self.name

        m = hashlib.sha256()
        m.update(self.name.encode())
        m.update(CFG().make_slug().encode())
        m.update(case.get_fpp(self, False).encode())

        if case.params.get('chemistry', 'F') == 'T':
            m.update(case.get_cantera_solution().name.encode())

        return m.hexdigest()[:10]

Yes it seems there is no way to avoid this, so long as Pyro is generating our code on the fly for each chemistry case. So yes I am OK with this, as we should probably have a 'store' of mechanism files that we test with located somewhere in VC

Yes it seems there is no way to avoid this, so long as Pyro is generating our code on the fly for each chemistry case. So yes I am OK with this, as we should probably have a 'store' of mechanism files that we test with located somewhere in VC

I agree.  This is why you can run these example cases without downloading a mechanism file.

Benchmarking is failing with

      .=++*:          -+*+=.        |<EMAIL_ADDRESS>[Linux]
     :+   -*-        ==   =* .      | -------------------------------------------------------
   :*+      ==      ++    .+-       |
  :*##-.....:*+   .#%+++=--+=:::.   | --jobs 24
  -=-++-======#=--**+++==+*++=::-:. | --mpi --no-gpu --no-debug --no-gcov --no-unified
 .:++=----------====+*= ==..:%..... | --targets pre_process, simulation, and post_process
  .:-=++++===--==+=-+=   +.  :=     |
  +#=::::::::=%=. -+:    =+   *:    | ----------------------------------------------------------
 .*=-=*=..    :=+*+:      -...--    | $ ./mfc.sh (build, run, test, clean, count, packer) --help

 Benchmarking pre_process, simulation, and post_process (build/benchmarks/e180):

   1/4: 5eq_rk3_weno3_hllc @ benchmarks/5eq_rk3_weno3_hllc/case.py

     > Log:     build/benchmarks/e180/5eq_rk3_weno3_hllc.out
     > Summary: build/benchmarks/e180/5eq_rk3_weno3_hllc.yaml
     $ ./mfc.sh run /storage/scratch1/6/sbryngelson3/runners/actions-runner-4/_work/MFC/MFC/pr/benchmarks/5eq_rk3_weno3_hllc/case.py --case-optimization --targets pre_process simulation post_process --output-summary /storage/scratch1/6/sbryngelson3/runners/actions-runner-4/_work/MFC/MFC/pr/build/benchmarks/e180/5eq_rk3_weno3_hllc.yaml -c phoenix --gpu -g 0 1 -n 2 -- 1

Error: Failed to load YAML from "/storage/scratch1/6/sbryngelson3/runners/actions-runner-4/_work/MFC/MFC/pr/build/benchmarks/e180/5eq_rk3_weno3_hllc.yaml": [Errno 2] No such file or directory: '/storage/scratch1/6/sbryngelson3/runners/actions-runner-4/_work/MFC/MFC/pr/build/benchmarks/e180/5eq_rk3_weno3_hllc.yaml'

Terminated

mfc: ERROR > main.py finished with a 143 exit code.
mfc: (venv) Exiting the Python virtual environment.

@sbryngelson Thanks for letting me know. Just fixed it.

@henryleberre mind pulling from upstream for 'cleanliness'? I made an update there yesterday and want to see how it worked out.

@henryleberre pull my 'fix cleanliness' PR so it's up to date with master if possible (that's the thing I would like to test).

@henryleberre pull my 'fix cleanliness' PR so it's up to date with master if possible (that's the thing I would like to test).

Yes I did, that was the force-push you saw (a rebase on upstream). This is the workflow step that is causing it to fail:
https://github.com/henryleberre/ChemMFC/blob/23df5d6551e7a82217b3259272efe7075d1cf30a/.github/workflows/cleanliness.yml#L99-L103

Testing this is a bit harder because it only appears meaningfully in multi-rank cases. Once you're done, can you test for correctness on a longer 4-rank case, for example, to be sure there isn't a subtle mistake?

Also @henryleberre: It's somewhat unclear what all the different variables really mean (and they don't seem to have comments). I think even after refactor we have 2 or 3 versions of this variable. Can we put comments to say what those are/why they exist?

Testing this is a bit harder because it only appears meaningfully in multi-rank cases. Once you're done, can you test for correctness on a longer 4-rank case, for example, to be sure there isn't a subtle mistake?

Also @henryleberre: It's somewhat unclear what all the different variables really mean (and they don't seem to have comments). I think even after refactor we have 2 or 3 versions of this variable. Can we put comments to say what those are/why they exist?

I did "document" these like this:
    ! Cell Indices for the interior points (O-m, O-n, 0-p).
    type(int_bounds_info) :: idwint(1:3)
    !$acc declare create(idwint)

    ! Cell Indices for the entire domain, including the buffer region.
    type(int_bounds_info) :: idwbuff(1:3)
    !$acc declare create(idwbuff)

I could maybe have specified these were the proc's local domain. They are only defined in one place now so it's very straightforward to understand. The other indices are module-specific and I'm not sure I have enough context to explain them better than the code does.

Testing this is a bit harder because it only appears meaningfully in multi-rank cases. Once you're done, can you test for correctness on a longer 4-rank case, for example, to be sure there isn't a subtle mistake?

I'm not sure how this is true. Why are the 2-rank test cases not enough?

Testing this is a bit harder because it only appears meaningfully in multi-rank cases. Once you're done, can you test for correctness on a longer 4-rank case, for example, to be sure there isn't a subtle mistake?

I'm not sure how this is true. Why are the 2-rank test cases not enough?

2-rank may be enough, but I'm not sure if our current single test case in the suite is enough.

Testing this is a bit harder because it only appears meaningfully in multi-rank cases. Once you're done, can you test for correctness on a longer 4-rank case, for example, to be sure there isn't a subtle mistake?

Also @henryleberre: It's somewhat unclear what all the different variables really mean (and they don't seem to have comments). I think even after refactor we have 2 or 3 versions of this variable. Can we put comments to say what those are/why they exist?

I did "document" these like this:
    ! Cell Indices for the interior points (O-m, O-n, 0-p).
    type(int_bounds_info) :: idwint(1:3)
    !$acc declare create(idwint)

    ! Cell Indices for the entire domain, including the buffer region.
    type(int_bounds_info) :: idwbuff(1:3)
    !$acc declare create(idwbuff)

I could maybe have specified these were the proc's local domain. They are only defined in one place now so it's very straightforward to understand. The other indices are module-specific and I'm not sure I have enough context to explain them better than the code does.

Ah I did not see this part. I mostly saw the module indices. I think those are still confusing. Why do we need any such variables beyond idwint and idwbuff? I can imagine perhaps a couple edge cases but that's about it (like when we do direction reshaping).

The debug cases are failing on this case (perhaps among others, they only run a fraction of the cases in CI):
 m_mpi_proxy.fpp:1289: @:ALLOCATE_GLOBAL(ib_buff_send(0:-1 + gp_layers * (m + 2*gp_layers + 1)* (n + 2*gp_layers + 1)* (p + 2*gp_layers + 1)/ (min(m, n, p) + 2*gp_layers + 1)))
 m_mpi_proxy.fpp:1301: @:ALLOCATE_GLOBAL(ib_buff_recv(0:ubound(ib_buff_send, 1)))
 m_ibm.fpp:106: @:ALLOCATE_GLOBAL(ghost_points(num_gps))
 m_ibm.fpp:107: @:ALLOCATE_GLOBAL(inner_points(num_inner_gps))
At line 567 of file /Users/runner/work/MFC/MFC/src/simulation/m_ibm.fpp
Fortran runtime error: Index '1' of dimension 1 of array 'ghost_points' above upper bound of -99999918
Error termination. Backtrace:
Could not print backtrace: executable file is not an executable
Could not print backtrace: executable file is not an executable
Could not print backtrace: executable file is not an executable
Could not print backtrace: executable file is not an executable
#0  0x10345ad2f
#1  0x10345b8d7
#2  0x10345bc8f
#3  0x1022f8bdf
#4  0x10230ad67
#5  0x102555d83
#6  0x1026ef79b
#7  0x1026efdfb
mfc: ERROR > :( /Users/runner/work/MFC/MFC/build/install/7e1418ef14/bin/simulation failed with exit code 2.

Case 725A3D56, F13DF273, and 0F6C4340 from the fraction that ran in CI here
https://github.com/MFlowCode/MFC/actions/runs/11660348744/job/32478811683?pr=653#step:7:5812


Talk about prompt injection and LLM security

This tutorial doesn't yet talk about the security implications of building software on top of LLMs - in particular the prompt injection class of vulnerabilities.
I think this is a problem. Prompt injection is a particularly nasty vulnerability, because if people don't understand it they are almost doomed to build systems that are vulnerable to it.
It also means that a lot of the obvious applications of generative AI are not safe to build. A personal assistant that can summarize and reply to your email for example - that's not safe, because there might be a prompt injection attack in one of the emails that it reads.
I wrote more about this here: https://simonwillison.net/2023/Apr/14/worst-that-can-happen/ (and in this series of posts).

Hey @simonw , great callout! We are working on an additional 4 lessons which includes one one prompt injection / security. Will make sure to point to some of the great content you have given to this community. Keeping this open until we deliver on this.

我就认可


JobRouter OpenAI integration PR

Purpose
Adding link to quickstart and fixing bug

...

Does this introduce a breaking change?

[ ] Yes
[X] No

Pull Request Type
Additive changes

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

How to Test

Get the code

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

Test the code

What to Check
Verify that the following are valid

...

Other Information

#sign-off

#sign-off


Can we move to Neovim and drop support for VIM?

Hi, since less and less people use VIM, can we move to Neovim only? I will re-write this plugin with lua, but the idea is still heavily hack into fzf.vim and leverage its script local utility functions.

I create a lua version here: https://github.com/linrongbin16/fzfx.nvim.

这个仓库可以维护现有的功能即可, 新的功能就在nvim下开发就行了


Consider adding stylelint-validator plugin

https://github.com/csstree/stylelint-validator

This is fixed by PR #13 and will be released as a part of the v2.0.0 milestone.

Reopened because of #16.

The stylelint-csstree-validator plugin isn't ready for prime time use with Sass yet, namely issues with calculated values and color functions.

Removing from v2.0.0 milestone.


chore(deps): upgrade dev dependencies

Upgrades project dependencies. See details in workflow run.

Automatically created by projen via the "upgrade-dev-dependencies-k8s-29-main" workflow

⚪ Backport skipped
The pull request was not backported as there were no branches to backport to. If this is a mistake, please apply the desired version labels or run the backport tool manually.
Manual backport
To create the backport manually run:
backport --pr 4398

Questions ?
Please refer to the Backport tool documentation


Change to public IDs for URLs instead of database row IDs

Generally as a good security thing, avoid enumeration, etc. Could use CoolID, a NanoID implementation, or we could use Sqids

We should also do this for federated actors, we definitely want to avoid enumeration there. Might be something for Federails.


Engine doesn't exit gracefully when an ÖbEngine object is not constructed with the right arguments

function Game.Start()
    local collider = obe.Collision.PolygonalCollider();
end

Will result in a crash while performing the purge's collectgarbage()

Issue opened here https://github.com/ThePhD/sol2/issues/1144

Second related issue : https://github.com/ThePhD/sol2/issues/1174


⚠ Bofrost.es has degraded performance

In d27acdf, Bofrost.es (https://www.bofrost.es) experienced degraded performance:

HTTP code: 200
Response time: 1830 ms

Resolved: Bofrost.es performance has improved in 366b3a8.


[US] General Clean-up and Testing of Scripts

As a platform developer, I want to create a usable file structure for scripting, so that accessing scripts is not a hassle.

MoSCoW
Risk
Story Points

Must have
low
2

Acceptance Criteria
All scripts are working properly, the file structure is clean and according to what is expected, and unecessary files are removed.
Risks
Scripts potentially not working for extended periods of time, but this risk is very low with adequate support
Tasks

[x] Change the file structure for all scripting (create a new script folder, put all necessary scripts for the platform in there)
[x] Make sure all scripts in the folder still work
[x] Remove the back-up of the old database from the repository
[x] Move "import_appconfig_from_json" out of the file it is currently in, into meta.sql (which should no longer be in the usecase folders)

Signing off!

https://github.com/WillTrem/UInnovate/assets/61199167/4781ed31-dd26-4f63-a67c-771f0f29c479


[es] Spanish translation

This PR includes the Spanish translation.

Thanks! I'll be updating the page itself later this evening :)


Remove ASM dep from ClassDescriptor

See https://github.com/stapler/stapler/pull/225#pullrequestreview-667138790. Unfortunately it does not seem to work consistently—the test passes using my IDE’s compile-on-save mode, but not via mvn clean test. If I am following correctly, the LocalVariableTable always includes the parameter names, which are then redundantly included in a MethodParameters section only when compiled with debug information? Need to look into why this would be. I suspect the difference is between bytecode and runtime retention.

I think the problem is that plugin-pom does not default to setting https://maven.apache.org/plugins/maven-compiler-plugin/compile-mojo.html#parameters so this information is unavailable. We could add this going forward, but then existing plugins would lack this information, and it could be years before they are all updated.
In some cases—web methods?—methods have associated *.stapler files which also contain the parameter names, but this does not apply to constructors and I do not think we can rely on it.

Or there is a *.stapler file for constructors? Sometimes?
DispatcherTest.testInheritance also fails: org/kohsuke/stapler/DispatcherTest/Inheritance/doBar.stapler is present but there is nothing for Inheritance2.

Confirmed, the following makes tests pass:
diff --git core/pom.xml core/pom.xml
index c03db644e..4dcd86d9a 100644
--- core/pom.xml
+++ core/pom.xml
@@ -23,6 +23,24 @@
         </executions>
       </plugin>
     </plugins>
+    <pluginManagement>
+      <plugins>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-compiler-plugin</artifactId>
+          <executions>
+            <execution>
+              <goals>
+                <goal>testCompile</goal>
+              </goals>
+            </execution>
+          </executions>
+          <configuration>
+            <parameters>true</parameters>
+          </configuration>
+        </plugin>
+      </plugins>
+    </pluginManagement>
   </build>
   <dependencies>
     <dependency>

Could probably be reworked to try MethodParameters first (the most efficient and supported system), then fall back to using ASM for LocalVariableTable if necessary.

Interesting, if you're using spring boot, their maven / gradle plugins automatically add that command line flag these days.
I've been using the flag for users for simplifying models that use jackson as you don't need to duplicate the field name in the annotation as it can read it from the constructor parameter name.
Shouldn't it be in compile as well? not just testCompile?

Shouldn't it be in compile as well? not just testCompile?

If we are talking about plugin-pom (and pom), then yes of course. I was just pointing out that for purposes of unit tests in Stapler it suffices to add that POM snippet to make tests pass from Maven.
Ideally the annotation processor that writes *.stapler files could be amended to skip this if it can detect that method parameters are being written, to avoid redundancy, assuming these files are used for nothing else (I would have to research it a bit).


Add chain column to all database tables (except User), convert primary keys to always include the chain id. So under most circumstances the primary key went from id -> id + chain. This ensures uniqueness across chains and allows for filtering for a specific chain across all tables.
We were using a globally defined networkConfig constant that inferred the running network via an env var env.CHAIN_ID. This approach no longer works as incoming requests can be defined for any network (or no network for listing queries). Convert networkConfig to networkContext, a request scoped context service. This also means that accessing networkContext in the global scope is no longer supported, any reference to it needs to be inside a request scope, or more specifically, needs to always be referenced within a function call. For example we used a pattern of new Service(networkConfig.address) in the global scope quite often, this approach is no longer valid. The approach across the repo has been refactored to support the new request scoped model.
Along the same lines as the previous comment, any usage of env.CHAIN_ID has been refactored to use networkContext.chainId.
Request scoped context is implemented using AsyncLocalStorage following the simple pattern described here: https://medium.com/trabe/asynclocalstorage-for-easy-context-passing-in-node-js-e33c84679516. Renamed userMiddleware to sessionMidleware and extended it to support pulling the ChainId from the request header and injecting it into the AsyncLocalStorage session context. Inside of NetworkContext, we pull the active chainId from the session context, and use this to resolve network specific config.
We isolate all network specific config into a single file in network/${network}.ts and register the model inside the network/network-config.ts file. The goal will be that adding support for a new network will only require the creation of this config file.
For the worker, we currently expect that there will be 1 worker per network, but this can quite easily be changed. I will leave it to @franzns to weigh in on how he'd prefer this to be managed, and we can configure accordingly.

This is just a rough first pass and needs extensive testing. We'll also need to develop a migration strategy for fantom and optimism.

Went through all the code and from a first glance looks good. Great work!
I will create an AWS deployment next week and check if I run into any problems.
As for the workers, the plan is to use one worker for all chains and see how that performs with a configuration option to specify which workers should handle which chains. I will keep the worker environment setup on AWS EBS using SQS for now. This can be redone to a dockerized/k8s deployment at a later time.

I have created a new branch v3-canary for this deployment.


POST(create) operation should post to a collection URI

It is a typical RESTful best-practice to POST/create to a collection/list URI rather than have the client specify an ID in the POST URI. This allows for the API provider to create the object instance and assign an instance ID. This URI of the created object (including ID) can be specified as the "location" http-header attribute. The created object instance can be returned in the body of the response.

Fixed in #121


Change AllUsers to be consistent with perMachine

Hi Jared,
I've been working on the GitHub Extension for Visual Studio. Because it's packaged with Visual Studio 2017, we need to set the AllUsers property in the VSIX to True (there is no user context when Visual Studio is installing).
Unfortunately this wrecks all kinds of havoc when trying to install a standalone VSIX file. Visual Studio seems to think it owns the extension and will happily and silently replace the contents of the VSIX with the previously installed version! If AllUsers is changed to False, suddenly everything seems to work fine (no more install/uninstall issues). See here: https://github.com/github/VisualStudio/issues/864
Because I don't necessarily want to deal with two separate VSIX flavors, I've tweaked VsixUtil to ensure that the AllUsers property is consistent with perMachine. AllUsers seems to be a flag that can force that can force perMachine to be False. See here:

When a package has AllUsers set to True, it will warn that this setting is being changed and continues on with a non-admin install.
Is this something you would be up for incorporating? 😄
I'll follow up with a PR for basic VS 2017 support...

I think you may be running into a similar problem that we're having on roslyn.  Is the Github extension an in the box / product extension now?

Is the Github extension an in the box / product extension now?

Yes, this is exactly it. It has been driving me crazy for the last few days. It's like it silently auto-repairs itself and restores the old version!
My main concern is for contributors and bug-reporters who might want to install an out-of-band VSIX. It's too easy to get VS 2017 into a state where it's impossible to install/uninstall the VSIX without completely deleting/reinstalling VS. I don't want to inflict this kind of pain on devs who have taken the time to contribute/test!
I'd be very interested to know what the problem is exactly and how you get around it with roslyn?

I've just rebased onto master and made the setting of AllUsers a little more robust.

I'd be very interested to know what the problem is exactly and how you get around it with roslyn?

I'm not sure there is a way to work around this unfortunately.  It's a problem for us as well as we can't give users VSIX patches for Roslyn ATM 😦
Does anyone from your team coordinate issues between Github and VS?  I can get them in touch with the right people for this issue.

If you have a moment, any chance you could have a look at this issue and see if it's familiar?
https://github.com/github/VisualStudio/issues/877
I noticed there were a bunch of Roslyn extensions with AllUsers=false (but InstalledPerMachine=true). Do you know if this changes whether or not the extensions will automatically update?

@tinaschrepfer got in touch earlier today and I'm hoping she will take a look. 😄

I'm a bit less familiar with the details.  @tannergooding though should be able to comment if this is hitting the same issue as Roslyn.

We have an internal tool which fixes things up (which Roslyn isn't consuming yet).
Basically, my understanding is:

Anything that ships in box should have AllUsers and InstalledByMSI set.
Anything that overrides something that ships in box should have Experimental set and should not have AllUsers or InstalledByMSI set.


Hotfix/fix build script

Closes #14 by fixing patch and submodule commands

The fix has been verified in three different environments: 1) poseidon server, 2) vmware vm, 3) public cloud service. Closing the PR.


CLOUDSTACK-9299: Out-of-band Management for CloudStack

Support access to a host’s out-of-band management interface (e.g. IPMI, iLO,
DRAC, etc.) to manage host power operations (on/off etc.) and querying current
power state in CloudStack.
Given the wide range of out-of-band management interfaces such as iLO and iDRA,
the service implementation allows for development of separate drivers as plugins.
This feature comes with a ipmitool based driver that uses the
ipmitool (http://linux.die.net/man/1/ipmitool) to communicate with any
out-of-band management interface that support IPMI 2.0.
This feature allows following common use-cases:

Restarting stalled/failed hosts
Powering off under-utilised hosts
Powering on hosts for provisioning or to increase capacity
Allowing system administrators to see the current power state of the host

For testing this feature, please install ipmitool (using yum/apt/brew) and ipmisim:
https://pypi.python.org/pypi/ipmisim
The default ipmitool location is assumed in /usr/bin, if this is different in your env please fix the global setting, see FS for details on various global settings.
FS:
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Out-of-band+Management+for+CloudStack
/cc @jburwell @swill @abhinandanprateek @murali-reddy @borisstoyanov

Local test results (the feature is hypervisor agnostic, integration test also run with Travis as well):
==== Marvin Init Successful ====
=== TestName: test_01_configure_oobm_invalid | Status : SUCCESS ===
=== TestName: test_02_configure_oobm_valid | Status : SUCCESS ===
=== TestName: test_03_enabledisable_oobm_invalid | Status : SUCCESS ===
=== TestName: test_04_enabledisable_oobm_valid | Status : SUCCESS ===
=== TestName: test_05_enabledisable_across_clusterzones_oobm_valid | Status : SUCCESS ===
=== TestName: test_06_oobm_issue_power_action | Status : SUCCESS ===
=== TestName: test_07_oobm_background_powerstate_sync | Status : SUCCESS ===
=== TestName: test_08_multiple_mgmt_server_ownership | Status : SUCCESS ===
=== TestName: test_09_oobm_change_password | Status : SUCCESS ===

UI Screenshots;

Pinging for review @jburwell @swill @wido @pyr @koushik-das @agneya2001 @DaanHoogland @rafaelweingartner @GabrielBrascher @kishankavala

I did an one hour code review and found nothing that will make me :-1:  it so; LGTM @swill I will however start integration tests and find more time as the size of the change warrants a two day review.
looks good @bhaisaab

@DaanHoogland thanks, @borisstoyanov will share QA results next week as well

Thanks guys.  I will try to get this one queued up for CI.

Really exciting PR @bhaisaab!
We'll pull this in for testing as well.

Thanks @swill @kiwiflyer

@swill I am running the tests on this one. You want to both run it or beat me at it?

First read-through didn't raise any eyebrows on my end. LGTM

Perfect thanks @DaanHoogland.  Once we get more people using bubble, maybe we can start posting to the PR that we are kicking off a CI run (like you did here) so we can be sure to best use our limited resources.  I have been helping @kiwiflyer and @dmabry get bubble setup in their environment and I think they are all set now, so they will likely be contributing to this effort as well.

ok, @swill reporting without upr. two manual retests did make it equal in effort.
all regular bubble tests passed on this except for the two I never get right the first time.
1502.results.network.txt
1502.results.vpc_router.txt
retests:
1502.results.routers_network_ops.txt
[dahn@blimbing templates]$ ssh<EMAIL_ADDRESS>Warning: Permanently added '<IP_ADDRESS>' (ECDSA) to the list of known hosts.
<EMAIL_ADDRESS>password:
ping -c 3 <IP_ADDRESS>
PING <IP_ADDRESS> (<IP_ADDRESS>): 56 data bytes
64 bytes from <IP_ADDRESS>: seq=0 ttl=46 time=24.063 ms
64 bytes from <IP_ADDRESS>: seq=1 ttl=46 time=65.177 ms
64 bytes from <IP_ADDRESS>: seq=2 ttl=46 time=23.749 ms
--- <IP_ADDRESS> ping statistics ---
3 packets transmitted, 3 packets received, 0% packet loss
round-trip min/avg/max = 23.749/37.663/65.177 ms
I also started the tests added in this PR, though ILO technology is not really relevant in a virtualised environment. Some but not all of them passed, I don't think it should keep us from merging this experimental feature in. I will look at the configuration changes in advanced.cfg to see if I can get them to run in the 2kvm bubble cfg. probably not all of them.
1502.results.outofbandmanagement.txt
@rhtyd maybe in a follow-up; can we split the test in  hardware required true and false?

@DaanHoogland thanks, in your test environment you'll need to install ipmitool (yum/apt-get) and the oobm tests would pass. These tests are also run by Travis, and see they pass too; in travis's before_install.sh we install/setup ipmitool see the changes for details

@DaanHoogland additional notes on how tests work; we've introduce a new dependency ipmisim that I wrote for this feature, ipmisim is both a tool and a library that simulates an ipmi 2.0 service; we use ipmisim to spin up fake ipmi bmcs and configure them to a host. The marvin test uses that so we can run the tests without explicitly requiring an actual iLo/iDRAC bmc. ipmisim: https://pypi.python.org/pypi/ipmisim/0.6

I added a few comments, but before I continue:

Why assume a IP-Address and not a hostname OR IP? I know enough IPMIs why are available through a DNS hostname
Why is so much passed as a String through the code instead of a native type like integer or inetaddress

My first look looks good, but I can't test this easily.

@wido thanks for the comments. It seemed easier to pass the arguments to the oobm subsystem, there is a immutable map of option,string being returned this is why all configuration arguments are string. Also, the driver would convert the port to a string to execute this in a forked process (that calls ipmitool). For this reason, all configs are passed as strings and left to how the driver would like to use them or interpret them.

==== Marvin Init Successful ====
=== TestName: test_oobm_background_powerstate_sync | Status : SUCCESS ===
=== TestName: test_oobm_change_password | Status : SUCCESS ===
=== TestName: test_oobm_configure_default_driver | Status : SUCCESS ===
=== TestName: test_oobm_configure_invalid_driver | Status : SUCCESS ===
=== TestName: test_oobm_disable_feature_invalid | Status : SUCCESS ===
=== TestName: test_oobm_disable_feature_valid | Status : SUCCESS ===
=== TestName: test_oobm_enable_feature_invalid | Status : SUCCESS ===
=== TestName: test_oobm_enable_feature_valid | Status : SUCCESS ===
=== TestName: test_oobm_enabledisable_across_clusterzones | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_cycle | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_off | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_on | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_reset | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_soft | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_status | Status : SUCCESS ===
=== TestName: test_oobm_multiple_mgmt_server_ownership | Status : SUCCESS ===

Thank you @DaanHoogland @pyr @wido for the review
@swill I think we've enough votes for this one, though le'ts wait for @borisstoyanov 's test results which he has told me he'll share soon
@jburwell do you want to review and suggest any changes? thanks.

@rhtyd maybe in a follow-up; can we split the test in hardware required true and false?

@DaanHoogland for this feature's integration test, we don't need an explicit hardware required arg since we launch an ipmi simulator using ipmisim within the test wherever needed to perform end to end ipmi/power-action testing. If I still to explicitly put the value, it would be False on all test cases within test_outofbandmanagement.py

@rhtyd got ya. It will work like this but then still some tests require the simulator which is hardware in a sense. I'm just thinking, not commenting.

@rhtyd not really related to this PR, but a question I have.  How should we be tracking environment change requirements?  Ideally we would have a mechanism to flag this type of change.  I understand it is only needed for the tests to pass, so it won't be a blocker for people just using it, but I am wondering how we should be handling this stuff in general.  Do you have ideas?  Maybe we can try to call it out in the initial PR or something like that?  Ideally this info would show up in the release notes as well.  What do you think @pdion891?

I think this PR is in pretty good order.  I will let @jburwell have a look at it to see if he has any comments because it is a large PR, but pending any of his feedback, I think we are set on this one...

@swill,what are those changes you are refering too? are they required for regular deployment or just for tests? anyway we will need some info for the release notes has it seams like very cool new feature!
look like we will need documentation for the admin guide too which is almost done from this PR comments!Cool!

@pdion891 sorry I kind of just threw you into this one.  In this case there is a new requirement added for tests integration tests to pass, so it is not a blocking change, but I do think it is worth noting somewhere that new requirements were added for integration tests to pass.  More generally, I have not seen any mechanism with which we track environment change requirements introduced in PRs that are then presented to the user in some way.  I am assuming it makes sense that this data be in the release notes, but we need a way to flag the changes so they are easily discovered by the person who is generating the release notes (you in this case).  :)  Suggestions?

shouldn't we have a wiki page explaining how to do tests agains cloudstack? and this would be part of this page or a subpage ?

@swill @pdion891 thanks, I don't know of any format or wiki/document where we keep this thing. Each feature/marvin-test may require test specific environment so I'm not sure if we can have a general solution.
In general, for writing marvin tests new dependencies maybe added to marvin itself (like for this PR, ipmisim dependency is added to marvin, so anytime you install it you automatically get the dependency necessary by your Python based test). Along with this, you may need any external dependencies added to packaging requirements/dependency (for example, ipmitool added to debian control, while for rpms ipmitool was already in the list of dependencies for cloudstack-management package). For running tests, we should expect the test to work out of the box provided the dependencies are installed. For this PR, I've got ipmisim added to marvin's setup.py, and for Travis specific there is a apt-get install ipmitool. After this the oobm marvin test runs like any other test and internally is responsible for setting up (fake) ipmi server etc.

@swill I am going to start in my review later this evening.  Sorry for the delay.

@rhtyd ya, I am not too worried about this PR.  I think we are all set with this PR once @jburwell has a chance to do a quick once through.  This PR just prompted me to ask the question more generally.  This is probably not the right place for this conversation, but I wanted to see if there was an approach we should be taking going forward.

PR is code complete with testing efforts that @borisstoyanov can share
tag:mergeready

Marvin tests where successful. Testing community submission of OOBM, which includes end to end regression testing with IPMI simulator. Issue power state commands and stage transition, configuration and password management and testing of feature enablement on separate layers of infrastructure.
LGTM.
==== Marvin Init Successful ====
=== TestName: test_oobm_background_powerstate_sync | Status : SUCCESS ===

=== TestName: test_oobm_change_password | Status : SUCCESS ===

=== TestName: test_oobm_configure_default_driver | Status : SUCCESS ===

=== TestName: test_oobm_configure_invalid_driver | Status : SUCCESS ===

=== TestName: test_oobm_disable_feature_invalid | Status : SUCCESS ===

=== TestName: test_oobm_disable_feature_valid | Status : SUCCESS ===

=== TestName: test_oobm_enable_feature_invalid | Status : SUCCESS ===

=== TestName: test_oobm_enable_feature_valid | Status : SUCCESS ===

=== TestName: test_oobm_enabledisable_across_clusterzones | Status : SUCCESS ===

=== TestName: test_oobm_issue_power_cycle | Status : SUCCESS ===

=== TestName: test_oobm_issue_power_off | Status : SUCCESS ===

=== TestName: test_oobm_issue_power_on | Status : SUCCESS ===

=== TestName: test_oobm_issue_power_reset | Status : SUCCESS ===

=== TestName: test_oobm_issue_power_soft | Status : SUCCESS ===

=== TestName: test_oobm_issue_power_status | Status : SUCCESS ===

=== TestName: test_oobm_multiple_mgmt_server_ownership | Status : SUCCESS ===

I think this one is ready to merge...

@jburwell I will be doing a group of merges in a couple of hours which will include this one.  If you have any pending concerns with this one, speak up soon.  :)

@swill I have gotten diverted from review.  Hoping to pick up today.

@jburwell no worries, I know how that goes.  :)  Thanks for your support regardless...

@jburwell I've fixed based on your review, please re-review and advise further improvements, thanks
Integration test after last PR review changes:
=== TestName: test_oobm_background_powerstate_sync | Status : SUCCESS ===
=== TestName: test_oobm_change_password | Status : SUCCESS ===
=== TestName: test_oobm_configure_default_driver | Status : SUCCESS ===
=== TestName: test_oobm_configure_invalid_driver | Status : SUCCESS ===
=== TestName: test_oobm_disable_feature_invalid | Status : SUCCESS ===
=== TestName: test_oobm_disable_feature_valid | Status : SUCCESS ===
=== TestName: test_oobm_enable_feature_invalid | Status : SUCCESS ===
=== TestName: test_oobm_enable_feature_valid | Status : SUCCESS ===
=== TestName: test_oobm_enabledisable_across_clusterzones | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_cycle | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_off | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_on | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_reset | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_soft | Status : SUCCESS ===
=== TestName: test_oobm_issue_power_status | Status : SUCCESS ===
=== TestName: test_oobm_multiple_mgmt_server_ownership | Status : SUCCESS ===

Please force push to kick off travis again.  Thanks...

BTW, travis is showing exceptions in OOBM now, so you probably want to review those...

@swill the issue is actually with ipmitool binary itself https://bugzilla.redhat.com/show_bug.cgi?id=1286035  where it may sometimes fail due to errors in ipmitool's handling of sessions. I'll see if I can work around that.

For the issue reported by Travis, I force it to install ipmitool version 1.8.16; but looks like even that version causes the same failure (some times).

So is this going to be another reason Travis fails randomly?  :(  If we know it is going to fail randomly, we probably want to remove it from the Travis tests to make sure it does not create false negatives for everyone.  If it fails the tests in travis, does that mean that the functionality also randomly fails?

@swill I've added a Travis specific change and Travis specific patched ipmitool version (using patch from https://bugzilla.redhat.com/show_bug.cgi?id=1286035), I'm hoping with this the test should not cause random failure. If it happens to fail randomly in future, we can disable it from routine Travis runs.

OK. Sounds good. :) Thx...

@swill this PR is ready for CI test run and merge, thanks

I've just run final sanity tests on this PR, all the OOBM features are working and did not observed any side effects, so I'm happy with the latest changes getting merged.
LGTM!

@rhtyd @swill we are very close.  I have a couple of outstanding comments -- particularly around potential thread explosion in ProcessRunner.  Ultimately, I think the issues are straightforward to address.
/cc @borisstoyanov

@jburwell thanks, I'll sort them out. Meanwhile, if we can get the dynamic-roles PR merged today/tomorrow, I can use the annotations/validation usage in APIs and further improve the PR

Both this one and dynamic roles are queued up for CI as soon as I stabilize master.

CI RESULTS
Tests Run: 85
  Skipped: 0
   Failed: 1
   Errors: 0
 Duration: 6h 07m 49s

Summary of the problem(s):
FAIL: test_03_vpc_privategw_restart_vpc_cleanup (integration.smoke.test_privategw_acl.TestPrivateGwACL)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 265, in test_03_vpc_privategw_restart_vpc_cleanup
    self.performVPCTests(vpc_off, True)
  File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 332, in performVPCTests
    self.check_pvt_gw_connectivity(vm2, public_ip_2, vm1.nic[0].ipaddress)
  File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 559, in check_pvt_gw_connectivity
    "Ping to outside world from VM should be successful"
AssertionError: Ping to outside world from VM should be successful
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_FIVJNA/results.txt

Associated Uploads
/tmp/MarvinLogs/DeployDataCenter__May_06_2016_21_31_06_LPI7VW:

dc_entries.obj
failed_plus_exceptions.txt
runinfo.txt

/tmp/MarvinLogs/test_network_FIVJNA:

failed_plus_exceptions.txt
results.txt
runinfo.txt

/tmp/MarvinLogs/test_vpc_routers_WUHEVI:

failed_plus_exceptions.txt
results.txt
runinfo.txt

Uploads will be available until 2016-07-07 02:00:00 +0200 CEST
Comment created by upr comment.

@jburwell I've fixed the outstanding issues, please re-review and suggest any other improvement or LGTM. Thanks.

@rhtyd it looks like one of the OOBM smoke tests failed on the Travis build.  Could you please investigate?

@jburwell as mentioned in comments above, due to a bug in ipmitool itself (https://bugzilla.redhat.com/show_bug.cgi?id=1286035) some executions may fail. I'll add code to skip/workaround that known bug for the specific failing test, I had already added some code to skip test when the specific ipmitool bug was hit.

@jburwell can you do a final review, LGTM or share further improvements. Thanks.

@nvazquez can you check why test_03_list_snapshots failed in the Travis run, with tearDown exception.

@rhtyd sure, I'll examine this. This tests are introduced in #1497, I'll work on this

@rhtyd @kiwiflyer @swill a PR for fixing the problem #1539

@swill PR1539 passed Jenkins and Travis. After you merge it should resolve the issue in other PRs

@jburwell fixed the ProcessRunner issues, please do a final review and LGTM or suggest changes. Thanks.
@nvazquez @swill I've fixed two CI issues (Travis and Jenkins issues) in this PR as well

tag:mergeready
/cc @swill all green now

@swill @rhtyd LGTM based on code review

I see the following error which is causing the PR #1297 to fail.  Suggestions?
+---------------------------------------------+----------------------+--------+
| test_oobm_zchange_password                  | exceptions.Exception | 5.220  |
+---------------------------------------------+----------------------+--------+

Jenkins for #1537 is also being held up by these tests:
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.apache.cloudstack.outofbandmanagement.driver.ipmitool.IpmitoolWrapperTest
Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 300.337 sec <<< FAILURE! - in org.apache.cloudstack.outofbandmanagement.driver.ipmitool.IpmitoolWrapperTest
testExecuteCommands(org.apache.cloudstack.outofbandmanagement.driver.ipmitool.IpmitoolWrapperTest)  Time elapsed: 300.086 sec  <<< FAILURE!
java.lang.AssertionError: null
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertTrue(Assert.java:52)
	at org.apache.cloudstack.outofbandmanagement.driver.ipmitool.IpmitoolWrapperTest.testExecuteCommands(IpmitoolWrapperTest.java:112)

Results :
Failed tests:
  IpmitoolWrapperTest.testExecuteCommands:112 null

Tests run: 8, Failures: 1, Errors: 0, Skipped: 0

@swill thanks, will have a look at it

@swill I've tried to fix them here: https://github.com/apache/cloudstack/pull/1544


updated README to reflect updated oracle instantclient library versio…

…n, closes #1036

@cummingsm - note that I've also updated launchpad in fabber to reflect the new instantclient version.  If this fails, we'll need to revert that change as well.
https://github.com/gwu-libraries/fabber/commit/84ebff65ef4fbfa0881ff284423a33aea8fb4256


Log Dashboard - Feature

This is fairly awesome:
http://www.elasticsearch.org/overview/ see video
http://www.elasticsearch.org/videos/kibana-logstash/
Just FYI I found the above by way of: https://commando.io/ which is a beautiful complement to EE for people with multiple clients where you are hosting them as well as developing

How to Install/Setup Guide
ELK - https://miteshshah.github.io/linux/elk/
Logio.org - https://miteshshah.github.io/sysadmin/realtime-log-monitoring-in-your-browser/

+1 @MiteshShah Great stuff


Callback URL is different up to the API used

i have create a logic app with an HTTP trigger. In my code I try to get the url to call the logic app using the REST API, as documented here: https://docs.microsoft.com/en-us/rest/api/logic/workflowtriggers/listcallbackurl#workflowtriggercallbackurl
Unfortunately the url I get back is not the correct one.
I discovered the url is generated based on the api version I provide for the REST call.
Example:
Request:
az rest --method post --uri
https://management.azure.com/subscriptions/{{MYSUBSCRIPTIONID}}/resourceGroups/{{MYRESOURCEGROUP}}/providers/Microsoft.Logic/workflows/{{MYLOGICAPPNAME}}/triggers/manual/listCallbackUrl?api-version=2016-10-01
Response:
https://prod-97.westeurope.logic.azure.com:443/workflows/{{MYWORKFLOWID}}/triggers/manual/paths/invoke?api-version=2016-10-01&sp=%2Ftriggers%2Fmanual%2Frun&sv=1.0&sig=AFptEVnCjOaMhDS9ErHnIhEnmRhO3hGY2iFOl7CSH9U"
If I do a request using the 2019-05-01 api version the trigger url contains 2019-05-01 like:
https://prod-97.westeurope.logic.azure.com:443/workflows/{{MYWORKFLOWID}}/triggers/manual/paths/invoke?api-version=2019-05-01&sp=%2Ftriggers%2Fmanual%2Frun&sv=1.0&sig=AFptEVnCjOaMhDS9ErHnIhEnmRhO3hGY2iFOl7CSH9U"
But the real and working trigger url (at the moment) has always an API version with 2016-06-01 in it, like:
https://prod-97.westeurope.logic.azure.com:443/workflows/{{MYWORKFLOWID}}/triggers/manual/paths/invoke?api-version=2016-06-01&sp=%2Ftriggers%2Fmanual%2Frun&sv=1.0&sig=AFptEVnCjOaMhDS9ErHnIhEnmRhO3hGY2iFOl7CSH9U
Why this behaviour?
I expect to get back the correct trigger url, regardless the API version I provide for retrieving it

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

ID: 8a18a5a6-0630-17aa-a2d1-686e575d7e22
Version Independent ID: 02b19cbb-7ff3-4f3f-06d1-8a0ccb4ad707
Content: Call, trigger, or nest logic apps - Azure Logic Apps
Content Source: articles/logic-apps/logic-apps-http-endpoint.md
Service: logic-apps
GitHub Login: @ecfan
Microsoft Alias: klam

@olandese Thank you for your feedback! We will review and update as appropriate.

@olandese With either API version set, the trigger URLs seem to work fine for me. Are you seeing any error?
Also, I believe its best to stick to the API Version mentioned in the docs.

What do you mean is working fine? did you try to call the trigger with the different api version?

@olandese I was able to trigger the logic app using the returned callback URL with both api versions.
Is there any error when you try to trigger it? If so, could you share the same.

@olandese Just following up here... Could you please share the error that you are seeing when trying to trigger the logic app using the URL returned if any?

@olandese Since we have not heard back from you we will now proceed to close this thread. If there are further questions regarding this matter, please tag me in your reply. We will gladly continue the discussion and we will reopen the issue.

Hell guys, sorry, I was busy and not able to take care of this.
I will check it now and let you know

just checked, you are right guys, the URL I get back using the different API version works!


Playnite support

Just discovered Playnite (and installed it inside the nonsteamlaunchers prefix) which greatly simplifies the craziness of thousands of games across dozens of services with actually working third-party integrations (unlike GOG Galaxy).
Font dependencies (not blocking, it just complains in the logs if not available):

Trebuchet MS
Consolas

Also, it crashes if ran in desktop app mode, required for configuration, but then gives the option to run in safe mode which works (might require --safestartup argument).
Fullscreen mode can be launched directly via dedicated exe providing a controller-friendly UI.
It looks like third party plugins won't work due some Powershell issue when running under proton (causing it to crash).
Relevant link: https://playnite.link/

@ergoithz yes I'm aware of those things and have tested extensively for play nite, but as of right now even if you were to set up everything correctly to get play items to run... how would you easily integrate play nite with steam on Linux? The only reason to run play items would be to run the games themselves from playnite or create Steam Shortcuts I've tried to do this with play nite and haven't found a "friendly" way yet for the steam deck if that makes sense. However I did find out that if you install Playnite like how you say, I found that BoilR (Windows) will find all of your games under the Play nite Option. This is a good thing BUT, they aren't correct in the directory and launch options in order to function properly for Steam on linux.

*play items =playnite auto correct


[CHM317]--km-language component

add km-language component

fix code review issues.  please review again.


Decimal recognized as value

What problems I want to solve?
Awesome Library
The decimal place is recognized as values e.g
1,200.00 is interpreted as 120000 which is wrong. Can there be a fix to this please

Hi @AdieOlami , thanks for the report and glad that you like it :)
Sorry for taking so long to answer.
What is the configuration of the formatter when this error happens? Can you check if hasDecimals isn't defined as false? It must be set as true, which is the default for it.
In case it isn't, can you please share more details around the problem you're facing?
e.g. the setup of both formatter and delegate, and more details on how it happens.
Thanks!

Hi @AdieOlami, again, sorry for taking so long to answer you here. Are you still facing any issues?

Hello @AdieOlami, I'll close this issue due to inactivity, but feel free to open new ones in case you have any questions or problems. Thanks!


Multi-layer glass transparency rendering regression

We received a report from a user that a model that displayed layers of glass has a rendering regression as of v0.9.0. The model in question can be found about half-way down the page on https://amadeus.design/en/discover/, where it is being rendered by <model-viewer> v0.7.2.
Before and after renders of the model follow:

v0.7.2
v0.9.0

The user suspected that our change to "truly transparent" backgrounds is at fault, and given that this seems blending related, that seems like a good guess to me.

I believe this is by design, as our rendering of transparent objects was quite broken in v0.7. I don't see anything wrong with the v0.9 one, simply that it's more opaque. This is probably because we used to not render the back sides of transparent objects at all; I imagine cutting the opacity in half would probably get it back to where it was. Unfortunately transparent rendering is iffy in all real-time renderers and there's still some debate even in glTF as to what the spec means.

@elalish while you are investigating, would you be willing to suggest a mitigation or workaround for the issue?

Marking this as wontfix for now.


Add Ionic Capacitor

Before adding all these MBs to repo we should run squoosh or some other optimizer.
sample splash png: 2.6MB -> (98% down) 38kB

Can you provide steps how these files were generated - sorry I did not look into ionic since weeks and I remember nothing. thx

Can you provide requirements and steps how these files were generated - sorry I did not look into ionic since weeks and I remember nothing. thx

So all files inside android and ios folders are autogenerated.
ionic cordova platform add android
ionic cordova platform rm ios
And I wouldn't change those files, I know there is some useless stuff in it - addressing some comments in the PR.

What was the reason for including these autogenerated files into the repo again? Is it because of unavoidable modifications we need to make to them later on?

What was the reason for including these autogenerated files into the repo again? Is it because of unavoidable modifications we need to make to them later on?

Yeah. Even though we could in theory just commit diffs, and overwrite generated files on postinstall script. But i am not sure if different version of ionic will generate the same files.


Failed to compile with Angular 7.

I am getting the following error while compiling my application.

ERROR in ./node_modules/ng2-eonasdan-datetimepicker/src/datetimepicker.directive.ts
Module build failed (from ./node_modules/@ngtools/webpack/src/index.js):
Error: C:\Users\Aman Purohit\Desktop\fcs-app\fcs-Xweb-main\node_modules\ng2-eonasdan-datetimepicker\src\datetimepicker.directive.ts is missing from the TypeScript compilation. Please make sure it is in your tsco
nfig via the 'files' or 'include' property.
The missing file seems to be part of a third party library. TS files in published libraries are often a sign of a badly packaged library. Please open an issue in the library repository to alert its author and as
k them to package the library using the Angular Package Format (https://goo.gl/jB3GVv).
at AngularCompilerPlugin.getCompiledFile (C:\Users\Aman Purohit\Desktop\fcs-app\fcs-Xweb-main\node_modules@ngtools\webpack\src\angular_compiler_plugin.js:767:23)
at plugin.done.then (C:\Users\Aman Purohit\Desktop\fcs-app\fcs-Xweb-main\node_modules@ngtools\webpack\src\loader.js:41:31)
at
at process._tickCallback (internal/process/next_tick.js:188:7)

Earlier I was using Angular 4 and it was working perfectly. My current config is as follows.

Angular CLI: 7.0.5
Node: 8.9.0
OS: win32 x64
Angular: 7.0.3
... animations, common, compiler, compiler-cli, core, forms
... http, language-service, platform-browser
... platform-browser-dynamic, router

Can someone guide what could be a quick fix for this problem?

I was able to get rid of this issue by adding the following in tsconfig.json of my angular project.
"include":[     "src/**/*",     "node_modules/ng2-eonasdan-datetimepicker/src/datetimepicker.directive.ts"   ]


Link to "Haptic Usage Page" is not working

Hi,
The link for the "Haptics Usage Page" in all KnownSimpleHapticsControllerWaveforms properties pages seems to be broken (BuzzContinuous, Click, Press, Release, and RumbleContinuous). Is this the correct one? https://www.usb.org/developers/hidpage/HUTRR63b_-_Haptics_Page_Redline.pdf
Thank you!

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

ID: 946c2e57-0f20-a2d1-4a15-54935bb3035e
Version Independent ID: 0aa9e8df-ed5d-af5b-bd87-bf132b7e6222
Content: KnownSimpleHapticsControllerWaveforms.Press Property (Windows.Devices.Haptics) - Windows UWP applications
Content Source: winrt-api-build/xml/Windows.Devices.Haptics/KnownSimpleHapticsControllerWaveforms.xml
Product: uwp
Technology: winrt-reference
GitHub Login: @Karl-Bridge-Microsoft
Microsoft Alias: kbridge

Thanks for reporting this. The links seem to be working now.


Angular stub replacements

This PR substitutes mock feature implementations with their actual counterparts in all components except for Labels and TaskCreationWizard.
Please note that a Deleted tab still does not work, since enabling it to work would require more model module extension (in particular, adding a respective Projection`), which is a matter of further PRs.
It also slightly extends the model by adding a Status field to TaskDetails message, and making a task explicitly Open upon creation.
Now, completed tasks are displayed like so:
.

@YegorUdovchenko,PTAL again.

@YegorUdovchenko, PTAL again,


RangeInputElement mouseup-event doesn't always provide the mouse position

https://github.com/bwu-dart/polymer_elements_demos/blob/d67f9a4e9d35dc9b1e4c7f1d5a1185da2e91dad0/web/google_castable_video/google_castable_video_demo.dart#L49-L49
When I drag the knob and release it, the mouseup event doesn't always provide a position.
In the demo linked above this usually happens when I move the know to an advanced position and then move it back nearly to the beginning of the video.

Weird, can you repro this outside of the demo as well? (I can't seem to)

Since this is just a normal element it seems like it would be a general dartium bug, but if it only repros inside that demo then maybe something else is going on.

Sorry, I added only here bwu-dart/polymer_elements_demos#8,  that I can repro in Chrome (Linux) as well.


Settings "Saved" notification displays in previously set language

If you go to the settings, change the language and save it, the notification is displayed in the previously set language. This is kind of tricky because not all the strings used in Busy are updated in all languages. If you see on Crowdin that the language is completely translated, it does not mean that you will have the whole interface translated. You need to check directly on GitHub.

Expected behaviour
The notification text should be in the currently saved language.
Steps to reproduce

Go to settings
Change language to any such that it has the string "saved" updated on GitHub. (English, Czech, Hungarian, Arabic, ...)
Save settings
Repeat from step 2

Open Source Contribution posted via Utopian.io

Duplicate of #986. Sorry guys


patch: Redirect Core Concept to Handbook Glossary

Fixed Odyssey DT image and added comments for the redirect
Signed-off-by: Vipul Gupta (@vipulgupta2048) <EMAIL_ADDRESS>

Please make sure to read the CONTRIBUTING document before opening the PR for relevant information on contributing to the documentation. Thanks!

@balena-ci I self-certify!


[FEATURE] Include a few more abbreviations

Description
Consider appending the abbreviations on the homepage to the JSON database
Screenshots
No response
Additional information
No response

Do you want to take this issue?

Do you want to take this issue?

Sure!


feat: add more plugin APIs and documents

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

[x] N
[ ] Y

feat: add more plugin APIs and documents

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

0 of 8 (0.0%)  changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.02%) to 32.938%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

pkg/plugin/spec.go
0
8
0.0%

Totals

Change from base Build<PHONE_NUMBER>:
-0.02%

Covered Lines:
5660

Relevant Lines:
17184

💛  - Coveralls


Build AppX bundling launcher + client for Microsoft Store

[ ] Find and check documentation on how to submit an AppX to the store
[ ] Implement a way of building an AppX file/installer that contains the launcher + client
[ ] The launcher should only consume the client that came int the same AppX

From Electron docs:
https://www.electron.build/configuration/appx#how-to-publish-your-electron-app-to-the-windows-app-store

In the end it seems we don't need this since our normal launcher was approved.


Attestation

Attest the binaries and packages from the build artifacts.
Update .gitignore to ignore MSBuild binary log files and code coverage files.

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

Project coverage is 86.00%. Comparing base (4eddea9) to head (bf3b295).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2869   +/-   ##
=======================================
  Coverage   86.00%   86.00%
=======================================
  Files          75       75
  Lines        2829     2829
  Branches      459      459
=======================================
  Hits         2433     2433
  Misses        396      396

Flag
Coverage Δ

Linux
?

Windows
?

macOS
86.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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


Unable to reference bicep modules in an acr in different subscription in a different tenant

Bicep version
Bicep CLI version 0.16.2 (de7fdd2b33)
Unable to reference bicep modules in an acr in different subscription in a different tenant
To Reproduce
Try to reference bicep modules in an acr (which you have owner access too) in a different tenant and a different subscription.
Additional context
Error BCP192: Unable to restore the module with reference "br:.azurecr.io/bicep/modules/microsoft.resources/resourcegroups:v1": Service request failed.
Status: 401 (Unauthorized)
Content:
{"errors":[{"code":"UNAUTHORIZED","message":"authentication required, visit https://aka.ms/acr/authorization for more information."}]}
Any ideas on how to solve this issue?

Currently I believe Azure Lighthouse is the only way. Hopefully with #4947 we can use acr tokens instead of light house.

Closing as a duplicate of #5030.


Add link explaining the Safari workaround

https://github.com/PolymerLabs/actor-boilerplate/blob/master/src/bootstrap.ts#L25
Got an issue or PR that explains why this is needed?

it says right above. Or are you looking for something more in-depth? It's necessary to keep the worker alive. Safari seems to kill workers that don't receive messages.

Right, I saw that. I was looking for more info. Just curious about the behavior.

There really isn’t much more info than that :D We were just wondering why our worker suddenly stopped polling idb and it seems that Safari assumes that if a worker doesn’t receive or send messages after ~3 seconds, it’s superfluous and kills it.
Sending messages, even if there’s no message handler in the worker, seems to prevent Safari from killing it.

Welp, the good news is that we're creating the documentation now!


OnInstallPerCompany not working

Hello,
when I deploy my package to Business Central (Cloud) the OnInstallPerCompany is not getting triggered.
Any ideas?
codeunit 50100 InstallCodeunit
{
Subtype = Install;
trigger OnInstallAppPerCompany()
var
ReportSelection: Record "Report Selections";
begin
ReportSelection.RESET;
ReportSelection.SetRange(Usage,ReportSelection.USAGE::"S.Invoice");
If ReportSelection.FINDFIRST THEN repeat
ReportSelection.DELETE(TRUE);
UNTIL ReportSelection.NEXT=0;
ReportSelection.SetRange(Usage,ReportSelection.USAGE::"S.Cr.Memo");
If ReportSelection.FINDFIRST THEN repeat
ReportSelection.DELETE(TRUE);
UNTIL ReportSelection.NEXT=0;
  ReportSelection.Init();
  ReportSelection.VALIDATE(Usage,ReportSelection.Usage::"S.Invoice");
  ReportSelection.VALIDATE(Sequence,'1');
  ReportSelection.VALIDATE("Report ID",50101);
  ReportSelection.INSERT;
  ReportSelection.Init();
  ReportSelection.VALIDATE(Usage,ReportSelection.Usage::"S.Cr.Memo");
  ReportSelection.VALIDATE(Sequence,'1');
  ReportSelection.VALIDATE("Report ID",50102);
  ReportSelection.INSERT;

end;
}
best regards,
Thomas Barbut

Hello,
It is indeed an upgrade.
Thanks for the help. This solved my issue!
best regards,
Thomas Barbut

Glad to help.

I'm getting the same issue although I haven't changed the version of app. Tried uninstalling and un-publishing the app before publishing again but still it didn't work.
`codeunit 70112320 "App Installation"
{
Subtype = Install;
trigger OnInstallAppPerDatabase();
var
    myAppInfo: ModuleInfo;
begin
    InitStripeData();
    NavApp.GetCurrentModuleInfo(myAppInfo); // Get info about the currently executing module
    if myAppInfo.DataVersion = Version.Create(0, 0, 0, 0) then // A 'DataVersion' of <IP_ADDRESS> indicates a 'fresh/new' install

        PublishWebServices()
    else
        PublishWebServices();
end;

local procedure PublishWebServices()
var
    WebServiceRec: Record "Web Service Aggregate";
begin
    if not WebServiceRec.Get(WebServiceRec."Object Type"::Codeunit, 'DataExport') then begin
        with WebServiceRec do begin
            Init();
            "Object Type" := "Object Type"::Codeunit;
            "Object ID" := 70112322;
            "Service Name" := 'DataExport';
            Published := true;
        end;
        WebServiceRec.Insert(true);
    end;

    if not WebServiceRec.Get(WebServiceRec."Object Type"::Codeunit, 'ImportData') then begin
        with WebServiceRec do begin
            Init();
            "Object Type" := "Object Type"::Codeunit;
            "Object ID" := 70112323;
            "Service Name" := 'ImportData';
            Published := true;
        end;
        WebServiceRec.Insert(true);
    end;
end;

local procedure InitStripeData()
var
    StripeSetup: Record "StripeSetup";
begin
    StripeSetup.RefreshData();
end;

}`

What about this code doesn't seem to be working/firing? If you have done an uninstall/reinstall then this code should most definitely be firing.
Are you just not seeing entries in the web services table after performing these inserts? Or are you expecting different behavior? Note that since the services are being inserted here in code, they will not be removed when the extension is uninstalled
Web services should actually be included in an xml file inside of the extension, see Exporting data for Extensions

The webservices aren't being created in the first place. Same goes for the Stripe data. I have tried to publish/install app with a different name/id after removing the earlier version but still it doesn't work.

I'm currently getting a similar error message, while executing a get webservice call, during the installation routine. I'm doing this to get my stripe products
The error message says that "Publishing app fails due to "callback" functions".
When i comment out the installation code everything works fine.

I'm definitely not able to reproduce this issue as of yet.
Could you try a sanity check test and just throw an error inside of the code?
Something like the following codeunits would assert the path of execution. You should receive an error indicating where/what was code was fired.
codeunit 70112320 UpgCodeunit
{
    Subtype = Upgrade;

    trigger OnUpgradePerCompany()
    begin
        Error('ERRROR In UpgPerComp');
    end;

    trigger OnUpgradePerDatabase()
    begin
        Error('ERRROR In UpgPerData');
    end;
}

codeunit 70112321 InstallCodeunit
{
    Subtype = Install;

    trigger OnInstallAppPerCompany()
    begin
        Error('ERRROR In InstPerComp');
    end;

    trigger OnInstallAppPerDatabase()
    begin
        Error('ERRROR In InstPerData');
    end;
}


Update Best Practices doc for Github API Conditional Requests to mention limitations

Code of Conduct

[X] I have read and agree to the GitHub Docs project's Code of Conduct

What article on docs.github.com is affected?
https://docs.github.com/en/rest/using-the-rest-api/best-practices-for-using-the-rest-api?apiVersion=2022-11-28#use-conditional-requests-if-appropriate
What part(s) of the article would you like to see updated?
The section in this doc "Use conditional requests if appropriate" explains how to use conditional requests to avoid rate limit concerns for repeated use. However there's a problem with this approach which isn't mentioned in the doc - this solution only works when the access token being used to make the REST API doesn't change. When a different access token is used a different etag is returned, and further setting the if-none-match header for an etag value given with a previous token doesn't give the 304 response.
This is particularly a problem when using a GitHub App since the "installation token" refreshes (at least) every hour. Therefore this "best practice" will not be appropriate in many scenarios when using a GitHub App.
I suspect changing the API behavior will be hard or may never happen, so I think it's worth at least updating the document. I had made an assumption that conditional requests would work during the design of a GitHub App - based on this doc - and now that I know that they don't I have to rethink a few things.
Additional information
The overall issue with the API was raised in 2023 here by someone else - https://github.com/orgs/community/discussions/75228#discussioncomment-10697940 .

@mikebroberts Thank you for raising this issue! I'll get this triaged for review :sparkles: Our team will provide feedback regarding the best next steps for this issue - thanks for your patience! 💛

@mikebroberts Thank you for your patience while our SME team reviewed! Your analysis is correct 💛 Would you be open to raising a PR to call out this behavior with Apps?

I believe this Issue should remain open as I did not find this mentioned in the existing docs (linked above).
Getting this called out in the documentation in some way would likely save application authors a LOT of headache 😁 (perhaps speaking from personal experience).

@JoshuaTheMiller Thanks for the nudge on this! ✨ I've just reopened - would you be willing to raise a PR calling out this behavior with Apps? If not, we can look at opening this up to the community 💛

Rem


[Feature Request] Implement event on the "take photo" button.

I need to execute code in the same moment than i take the photo, by now i only can acept the preview and go back to my code, that can take a few seconds which would falsify the data collection of the accelerometer and the gyroscope.

why not just pass in a Command or a callback action?

No way to handle this as the preview is via the platform and we have no control.


Roe 2071 schema version field

JIRA link
https://companieshouse.atlassian.net/browse/ROE-2071
Change description
New enum to list schema versions.
New field typed by this enum in the submission model at the top level
Schema number taken from config and used to select version.
Logging updated to specify version.
Work checklist

[x] Tests added where applicable

Merge instructions
We are committed to keeping commit history clean, consistent and linear.  A BREAKING CHANGE can be part of commits of any type,
types other than fix: and feat: are allowed, for example build:, chore:, ci:, docs:, style:, refactor:, perf:, test:, and others,
footers other than BREAKING CHANGE: <description> may be provided.

There's a code smell to address too @mattch1 .

Change description also needs updating - Schema number no longer taken from config.


Allow the user to specify when TMS images are downsampled

This PR addresses #2360
We currently downsample TMS images when their width or height is > 1024 pixels.  This is currently hard-coded.
Sometimes we want to downsample when the image size is larger don't want to downsample the image at all.  This PR allows the user to specify the threshold at which we downsample TMS images in the Image renderer.

Allowing the user to change the image resolution by typing in a magic number is a pretty clunky interface.

I agree, but this is a result of how the image renderer is written.  See line 514 of ImageRenderer.cpp on master.
I toyed with a version of this code that simply turns downsampling on or off, but in some cases we may still want to downsample, but a higher threshold, for performance reasons.  This PR still provides that option, while eliminating the hard coded magic number.

How about having the code determine how many LODs are present and simply providing the user with a slider that changes the currently selected LOD?

I thought about this too, but it gets tricky.
The LOD of the TMS image is determined within the GeoImageTMS class by the function GeoImageTMS::_getBestLOD().
This get called within the pure virtual GeoImage::GetImage() function, which calculates the LOD with the clunky variable (maxWidthReq, maxHeightReq) which on master is hard coded to 1024.
The first challenge is that the GeoImage classes have no access to the params database, so they can't look up a user defined value.
The second challenge is that if we were to pass a new LOD variable into the pure virtual GeoImage::GetImage() function, we would need to rewrite the function in the GeoImage base class and all subclasses.  There are other ways around this, but everything I can think of seems kludgy.
I hope that all makes sense.  Maybe we can follow up in today's meeting.

Allowing the user to change the image resolution by typing in a magic number is a pretty clunky interface.

I agree, but this is a result of how the image renderer is written. See line 514 of ImageRenderer.cpp on master.
I toyed with a version of this code that simply turns downsampling on or off, but in some cases we may still want to downsample, but a higher threshold, for performance reasons. This PR still provides that option, while eliminating the hard coded magic number.

How about having the code determine how many LODs are present and simply providing the user with a slider that changes the currently selected LOD?

I thought about this too, but it gets tricky.
The LOD of the TMS image is determined within the GeoImageTMS class by the function GeoImageTMS::_getBestLOD().
This get called within the pure virtual GeoImage::GetImage() function, which calculates the LOD with the clunky variable (maxWidthReq, maxHeightReq) which on master is hard coded to 1024.
The first challenge is that the GeoImage classes have no access to the params database, so they can't look up a user defined value.
The second challenge is that if we were to pass a new LOD variable into the pure virtual GeoImage::GetImage() function, we would need to rewrite the function in the GeoImage base class and all subclasses. There are other ways around this, but everything I can think of seems kludgy.
I hope that all makes sense. Maybe we can follow up in today's meeting.

I agree it may take some restructuring of the code to support easy-to-use LOD specification. So be it.

@clyne - This is ready for re-review.  A couple notes:

The LOD range for NaturalEarth is {0:3}.  I set the current default to be 1, but I think this still looks grainy for Katrina and Duku.  We could calculate a better default based on the Box extents, or we could also bump the default LOD to 2.

We discussed using a slider for the LOD controller.  However I think that sliders should be used after a widget contains a certain threshold of options.  For example, we wouldn't want a slider if we are only presenting two or three numeric options to the user.  This is subjective but I think that 10 is a good number.  Since our packaged TMS files only have 4 options, I chose a combo box.

The function _tilePath has been changed to be a static inline function (TilePath) in GeoImageTMS.h

Two additional static inline functions (IsTMSFile and GetAvailableTMSLODs) have been added to GeoImageTMS.h.  This was done so that GeoImageTMS, and ImageSubtabs use the same logic to determine the information provided by these helper functions.

@clyne I've responded to your in-line comments.
I'll work on a better default value next.

@clyne - I've addressed your two remaining comments, and added a default setting that falls back on our old logic for determining the LOD for the TMS image.

I'd guess the true maximum LOD is not available from the manual selector. Note: it looks like GetAvailableTMSLODs returns the maximum LOD (starting from 0), not the number of LODs available.

You were right - thanks for catching this.

It dumps core when I load CAM/AMIP_0.25degree/cam5_1_amip_run2.cam2.h1.1979-01-01-00000.nc  (or any other global data set) and select the maximum LOD

It dumps core when I load CAM/AMIP_0.25degree/cam5_1_amip_run2.cam2.h1.1979-01-01-00000.nc (or any other global data set) and select the maximum LOD

The core dump is due to a memory allocation error, which occurs in ImageRender.cpp:
    // (Re)allocate space for verts
    //
    _nverts = width * height * 3;
    _sb_verts.Alloc(_nverts * 3 * sizeof(GLfloat));
    _sb_normals.Alloc(_nverts * 3 * sizeof(GLfloat));

By default 'width' and 'height' are set to the width and height of the texture, which was reasonable before this change, but will not work now. A simple fix is the following:
diff --git a/lib/render/ImageRenderer.cpp b/lib/render/ImageRenderer.cpp
index 0d69a7c4..62a87cad 100644
--- a/lib/render/ImageRenderer.cpp
+++ b/lib/render/ImageRenderer.cpp
@@ -209,8 +209,8 @@ int ImageRenderer::GetMesh( DataMgr *dataMgr,
// will be used to set the width and height of the mesh.
//
_getTexture(dataMgr);   // Ugh, this function is more than a get method...

          _vertsWidth = _texWidth;

          _vertsHeight = _texHeight;

          _vertsWidth = min(1024, _texWidth);

          _vertsHeight = min(1024, _texHeight);
          rc = _getMeshDisplaced(

Obviously the literal 1024 should be defined as a const such as MaxResamplingResolution.

Could you please move PTMSLODInput out of the Subtab file since those are all deleted in the refactor PR?

No problem.  Moved to PTMSLODSelector.h.  Hell of an acronym...


Other_services needs to be json

Update data parsing for other_services

data model requires JSON
UI is splitting text with commas as delimiter

In the current admin interface, you can store the other_services

@lkpanganiban, from our data model,

For the backend/API side this is already supported. We will just need to update the actual data.


Getting Command Name in Before Action

Hi there,
I just started with Go and this framework.
I'm trying to unify some code right now and would like to outsource a check I do in every sub-command except one.
I was thinking that I potentially can use the app.Before hook to and skip the check for the exception case. Unfortunatelly I haven't found a way to get the current command name.
Is the name available to the Before func?
Thanks
Jan

Hi @janwiemers,
The subcommand is determined after the Before function is called on the App (https://github.com/urfave/cli/blob/master/app.go#L234-L243) so unfortunately the command that would be executed (if any) is not available in the Before function.

How about the After function? I see the way the code is written currently that the After function call is deferred right before Before is called and therefore still prior to the Command being resolved, but this could be changed. Intuitively I would expect the After function context to have the same contents as the Command Action context.

@phinnaeus the command name is available on the cli.Context passed into the After function. The call is defered, but the context variable is bound to the outer scope and so changes to it (including populating the .Command) are available.

Example:
package main

import (
        "fmt"
        "os"

        "github.com/urfave/cli"
)

func main() {
        app := cli.NewApp()

        app.Commands = []cli.Command{
                {
                        Name:    "add",
                        Aliases: []string{"a"},
                        Usage:   "add a task to the list",
                        Action: func(c *cli.Context) error {
                                return nil
                        },
                        After: func(c *cli.Context) error {
                                fmt.Printf("command: %s\n", c.Command.Name)
                                return nil
                        },
                },
        }

        app.Run(os.Args)
}

➜  /tmp go run /tmp/tmp.go add
command: add

My bad, I tested with an older version. Thanks for clarifying.

Given that this is from last year, I think I'm comfortable closing it 🙂 feel free to re-open / open a new issue / comment in support if there's still interest here!

You can use the Args function, in the cli context, to figure out the command in the Before function.
function Before(cliCtx *cli.Context) error {
	if cliCtx.NArg() >= 1 && cliCtx.Args().First() == "your command" {
				return nil
	}
}

This worked for me, you use case may vary.


[Question] Are you guys open to adding screenshots of the elements in the demo to the readme page?

Hello team,
First and foremost ... thanks for releasing this! One of the first ui libraries that I am actually excited to try out and use on iOS. Definitely bookmarked. I've downloaded and tried the demo. I think this project may receive more traction if people saw the various elements available. I have no issue of doing the work if it aligns with what you guys want out of this project. I will simply take screenshots in the app and create a new markdown page or add to the main readme. I am open to the actual look and feel.
Please let me know if you'd like this PR or you have a team working on this already and it is not needed!
Thank you for all of the hard work! I look forward to future updates of this.
Cheers,
Alex

Hi Alex
Thank you for kind words - many people worked to make this project happen! We are working on updating our documentation on https://developer.microsoft.com/en-us/fabric#/controls/ios and pieces from it will trickle down to this repo as additional docs/screenshots. I appreciate your offer for help and will keep you in mind!
Thanks,
Vlad


[E621] MD5 lookup results in duplicate requests if image is not found.

Bug description
When Grabber can't find an image by MD5 under the e621 model it makes a duplicate request to the same endpoint with a different format value.
Steps to reproduce
Run image tagging/renaming operation based on images using e621 as a source.
Expected behavior
A single request is made, and if no results are found, that the next file is moved to.

Thank you for looking into this.

Indeed I see how a bug could make this happen. I currently don't have access to my computer but I'll take a look and make a new release as soon as I come back next week. :+1:


fix for PR #3144

added a missing definition of ReturnType in Cholesky.h

Lets see :)

@karlnapf since the trusty-viennacl build i still failing I am setting up viennacl and will try to reproduce the error locally in order to investigate what is going wrong.


cbuildgen should read version from the pdsc file

Hi
cbuildgen relies on the version information in local_repository.pidx,
but it should rather take the version information from the PDSC file.
Scenario with an issue:
I got a local pack already configured some time ago:
$ grep Example_xxx $CMSIS_PACK_ROOT/.Local/local_repository.pidx
  <pdsc vendor="STMicroelectronics" url="[file://localhost/C:\path\](file:///C:/path/)" name="Example_xxx" version="0.1.0"></pdsc>

I updated my local pack:
$ grep version STMicroelectronics.Example_xxx.pdsc
    <release version="0.1.1-alpha" date="2022-06-07">

$ csolution.exe list dependencies -s xxx.csolution.yml -c xxx.AC6+B-U585I-IOT02A

=> OK
$ csolution.exe convert -s ./xxx.csolution.yml
C:/path/xxx.AC6+B-U585I-IOT02A.cprj - info csolution: file generated successfully

=> OK
$ cbuild.exe ./xxx.AC6+B-U585I-IOT02A.cprj
info cbuild: cbuild: Build Invocation 0.9.1 (C) 2022 ARM
(cbuildgen): Build Process Manager 0.11.2 (C) 2022 ARM
M650: Command completed successfully.
error cbuild: cpackget was not found, missing packs cannot be downloaded

=> Message is normal in my side, cpackget is really not available.
But the issue is why cbuild needs to use cpackget ?
$ echo $?
0

=> 2nd issue but minor, cbuild should exit with non 0 value in my case
If I update manually local_repository.pidx file with the correct version,
cbuild command is now well calling cbuildgen as expected;

Hi Daniel
$ cbuild.exe  gpio-exti.AC6+B-U585I-IOT02A.cprj
info cbuild: Build Invocation 1.0.0 (C) 2022 ARM
(cbuildgen): Build Process Manager 1.0.0 (C) 2022 ARM
M654: Package<EMAIL_ADDRESS>was added to the list of missing packages.
M650: Command completed successfully.
info cbuild: missing packs must be installed, rerun cbuild with the --packs option

$ echo $?
0

Is it what is expected ?
=> I agree that error message is clearer, but
=> version from local_repository.pidx is still taken into account
=> exit code is still OK

@jeromecoutant Thanks for the feedback!
The new local repository handling implemented for csolution and cbuildgen cmake was indeed not applied to cbuildgen packlist, this is being fixed in the PR #442.
Concerning the exit code, this is actually different from the one that you reported previously when the cpackget was not available. This time it is being fixed in the PR #9.

@jeromecoutant Can you please test it with the latest CMSIS-Toolbox 1.1.0? Many thanks!
https://github.com/Open-CMSIS-Pack/cmsis-toolbox/releases/tag/1.1.0

@jeromecoutant I would like to remind you to validate the fix and comment accordingly/close the issue.

Hi
To be honest, we don't use any more local_repository.pidx file :-)
But I will try to make a test!


fix: do not run tests conditionally

This PR ensures that reviewers are not fooled by green check marks that should not exist by ensuring that comet does not run tests conditionally based on a diff.  A change in a teeny piece of the codebase can break things in other pieces of the codebase, thus:
        if: env.GIT_DIFF

is folly and dangerous.
Please see #2120 for a very detailed description of the issue and note that #2120 is my preferred remedy.
This is a mind hack
The green check mark makes all of us feel comfortable.  We want to merge stuff that earns the green check mark. The eyes have the optic nerve -- the highest bandwidth path to our brains where we make merge/don't merge decisions.   The CI system is currently designed to deceive our minds by presenting a green check mark at the wrong times.  It must be meaningfully earned.
I couldn't really figure out why this issue was largely hidden from the maintainership team but running tests conditionally is the precise reason.
While I prefer to merge a PR that runs the tests 40 times, and ensures they're not conditional, this is the more important of the two changes and removes the blinder from our brains.
This is a bug fix
the reason that this is a bug fix is that what we have here is a bug that causes other bugs
My thoughts while working on the run tests 40 times PR series
Over and over thinking how can they not know about this, what is wrong, the tests pretty much don't work and can't be relied upon to function. How is it that the maintenance team doesn't know about this?
This is how.
a commit might change 1% of the codebase, causing only 1/6th of the tests to run.
It could cause problems in the other 5/6th of the codebase, which relies on that 1% to function in some clearly defined manner.

PR checklist

[ ] Tests written/updated
[ ] Changelog entry added in .changelog (we use unclog to manage our changelog)
[ ] Updated relevant documentation (docs/ or spec/) and code comments
[ ] Title follows the Conventional Commits spec

Oh look the tests pass!
What changed between commits here?
nothing

https://github.com/cometbft/cometbft/actions/runs/7649974837/job/20845289857?pr=2123

note: there's now been another flake out, above.
If the code does not change, the result of running tests should not change.

....and another.  Maybe it will pass this time.  Maybe not.

Thank you for your contribution and the effort you've put into this pull request. I appreciate the intent of removing the conditional run.
However, I'm currently leaning towards not merging this PR. The reason is that removing the conditional run may lead to more frequent setup failures, especially in pull requests that only modify the documentation rather than the source code. There may be better solutions than running tests on every occasion to address non-deterministic failures.
The practical approach would focus on resolving the underlying issue of non-determinism in the tests. Doing so allows us to maintain a more reliable testing environment without necessarily resorting to unconditional test runs.


IO: match the signature for DispatchIO with dispatch_io_create

The DispatchIO constructor maps to dispatch_io_create.  The value on
Windows for the fd (which is actually a HANDLE) is being truncated
since dispatch_fd_t is actually Int not Int32 on Windows.  Use the
dispatch_fd_t to ensure that the right size is always passed along.

@swift-ci please test

With this change, it is now possible to write an echo server for Windows:
import Dispatch
import ucrt
import WinSDK

let rx: DispatchQueue = DispatchQueue(label: "rx", qos: .userInitiated)

let hIn: HANDLE = GetStdHandle(STD_INPUT_HANDLE)
if hIn == INVALID_HANDLE_VALUE { exit(EXIT_FAILURE) }

let io: DispatchIO = DispatchIO(type: .stream, fileDescriptor: Int(bitPattern: hIn), queue: rx) { error in
  guard error == 0 else { exit(EXIT_FAILURE) }
}
io.setLimit(lowWater: 1)
io.setLimit(highWater: Int.max)

while true {
  io.read(offset: 0, length: Int.max, queue: rx) { done, data, errorCode in
    guard errorCode == 0 else {
      print("IO Error \(errorCode)");
      exit(EXIT_FAILURE)
    }

    guard let data = data else { return }
    data.withUnsafeBytes { (ptr: UnsafePointer<CChar>) -> () in
      print(">> \(String(cString: ptr))")
    }
  }
}

/cc @MadCoder Would you mind taking a look if you have a moment?

Looks nice and straightforward to me.

Pull request


Update charts to use 1.3.2 Version of OS/OSD

Signed-off-by: Peter Zhu<EMAIL_ADDRESS>Description
Update charts to use 1.3.2 Version of OS/OSD
Issues Resolved
https://github.com/opensearch-project/opensearch-build/issues/1882
Check List

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

For any changes to files within Helm chart directories:

[x] Helm chart version bumped
[x] Helm chart CHANGELOG.md updated to reflect change

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

I see this error
is waiting to start: trying and failing to pull image, looks like the images are still not published to dockerhub.


Proposal: Add Indonesian Translation

Language
Indonesian
What improvement are you suggesting?
I would like to propose the addition of Indonesian (ISO 639-1 abbreviation: id). I am excited to encourage people to use and learn with p5.js like I did, and I believe that localizing some of its main resources - documentation, tutorials, guides - would be effective in making it more accessible.
For the outcome, I expect the website to have a complete translation of Indonesian available. In particular, I personally would prioritize the Get Started, Reference, and Learn pages.
As an effect, I hope that in the long term, learning p5.js would become less intimidating, easier, more accessible, and perhaps even more fun.
Would you like to work on the issue?
Yes!

Hi, thanks for your interest in translating the website into Indonesian. We saw similar interest in #1182 as well. In general all the new official translation of the website were done either as part of GSoC or Processing Fellowship, mainly because we believe translation is a pretty big project and we want to take into account long term maintenance and sustainability of the translation.
That being said you can certainly create unofficial translation of the site and publish it at a URL of your choosing and share the URL with us here and we'll include it as an unofficial translation link.

Okay, that's understandable. Thank you, I appreciate the response! Actually, I was surprised I didn't see that issue -- I've searched the issues to check whether anybody had suggested anything similar. I guess I wasn't thorough.
Re: GSoC/Processing Fellowship, perhaps this information could be added to [https://github.com/processing/p5.js-website/blob/main/contributor_docs/i18n_contribution.md](the i18n contribution guide).

Thank you @amuritna for the suggestion! We just updated the i18n contribution guide


Add "pushup routes" command

Introspect and print out the routes in the application to the terminal, addresses #77.

Closes #77


fix: Answer when receive a call

https://github.com/TECH7Fox/sip-hass-card/assets/42282908/76e3c085-b24b-49e8-b2ec-07e5607b3359


[Bug Report] The checkbox component generated by the render function cannot be sorted or cannot display animated changes

Element UI version
1.3.7
OS/Browsers version
win10 / chrome 58.0.3029.110 (64-bit)
Vue version
2.3.3
Reproduction Link
https://jsfiddle.net/mqhe2007/yrz5ozgL/
Steps to reproduce

如果通过“checked”属性设置默认选中，
点击“排序”按钮
通过“value”属性设置默认选中，
点击“排序”按钮，可以排序，但是无法改变选中效果

What is Expected?
选中的checkbox按新顺序排序
What is actually happening?
checkbox没有排序

First of all, you need to add key to disable caching. It doesn't matter if you're using render function or plain template, because plain template without keys also fails in the case: https://jsfiddle.net/9y5cojoj/
So updated demos of checked and value are: https://jsfiddle.net/yrz5ozgL/1/ , https://jsfiddle.net/yrz5ozgL/2/ . Note that when you're using value, you should listen to input event instead of change event.