ciao-down: add support to specify a workload file as workload

With this patch create command is able to create instances from
workload files specified as workload argument.
The custom workload file can be provided using 3 different schemas:

http
https
file

or throght its absolute or relative path

../../relative/path
/absolute/path

for example to create an instance from a file hosted in a remote server:
$ ciao-down create https://raw.githubusercontent.com/01org/ciao/master/testutil/ciao-down/workloads/fedora25.yaml

fixes #1366
Signed-off-by: Julio Montes<EMAIL_ADDRESS>

Coverage decreased (-0.01%) to 65.69% when pulling dd7efce0714bc6f3934f3885f1724b6d3c1dcd6e on devimc:topic/ciao-down/workloadsDir into 7a3b18c0f3fd83694937e98390ef160c5ce99253 on 01org:master.

@markdryan quick refinement, I think absolute paths (i.e. beginning with /) are much nicer than file URIs from a UX perspective for local paths.

@markdryan quick refinement, I think absolute paths (i.e. beginning with /) are much nicer than file URIs from a UX perspective for local paths.

Yes, agreed.  The problem is that if we allow normal paths this creates an ambiguity if no path is specified at all.  For example,
ciao-cli create test
Does test refer to a file in one of the workloads directories or a file in the current directory?  We would expect the former but users may expect the latter.  With URLs there is no ambiguity.  We can work around this but we need to decide what the correct behaviour is in this case and document it.  Perhaps if there's no path, we look in the current directory first and then in the workload directories.

Coverage increased (+0.004%) to 65.716% when pulling b3fa14d0286fa55a134cd636fe99d0c3ceb21cbd on devimc:topic/ciao-down/workloadsDir into 763e6263f2820ca003bc70de965341f60e03d7c0 on 01org:master.

@markdryan  done

Coverage decreased (-0.01%) to 65.699% when pulling b3fa14d0286fa55a134cd636fe99d0c3ceb21cbd on devimc:topic/ciao-down/workloadsDir into 763e6263f2820ca003bc70de965341f60e03d7c0 on 01org:master.

@markdryan  that's why I said absolute paths they by definition remove the ambiguity as they start with a /

@markdryan that's why I said absolute paths they by definition remove the ambiguity as they start with a /

But the problem is that once you start accepting absolute paths users may logically expect to use relative paths as they can with any other command.  This is where the confusion and ambiguity sneak in.  So we need to come up with some plan to address this.

@devimc Sorry, I should have started a review rather than making lots of small comments.  Overall, the approach looks good but I have lots of small comments.  Also, I have one design question?  Should we place downloaded workloads in ~/.ciao-down/workloads.

Also, I have one design question? Should we place downloaded workloads in ~/.ciao-down/workloads.

I think that is not needed, doing that we could override different workloads with the same name and I wouldn't like that ciao-down overrides my custom workloads

Coverage increased (+0.03%) to 65.754% when pulling 2d031f0a826200167ebe5928cd54a1f751b898a3 on devimc:topic/ciao-down/workloadsDir into 5907b795d3cf5b2f0d349a2f6ae476aee3f53cce on 01org:master.

We'll need to update the README as well.

Coverage increased (+0.02%) to 65.745% when pulling 22c76a210cbb8da685536b4394dd40d5361ba966 on devimc:topic/ciao-down/workloadsDir into 5907b795d3cf5b2f0d349a2f6ae476aee3f53cce on 01org:master.

Coverage increased (+0.02%) to 65.745% when pulling 22c76a210cbb8da685536b4394dd40d5361ba966 on devimc:topic/ciao-down/workloadsDir into 5907b795d3cf5b2f0d349a2f6ae476aee3f53cce on 01org:master.

@markdryan changes applied, thanks for your review

Coverage increased (+0.02%) to 65.75% when pulling 391c3a243c058814879bdc4759e0c779cf31f77b on devimc:topic/ciao-down/workloadsDir into 5907b795d3cf5b2f0d349a2f6ae476aee3f53cce on 01org:master.

Coverage increased (+0.02%) to 65.75% when pulling 391c3a243c058814879bdc4759e0c779cf31f77b on devimc:topic/ciao-down/workloadsDir into 5907b795d3cf5b2f0d349a2f6ae476aee3f53cce on 01org:master.

Coverage increased (+0.009%) to 65.736% when pulling 391c3a243c058814879bdc4759e0c779cf31f77b on devimc:topic/ciao-down/workloadsDir into 5907b795d3cf5b2f0d349a2f6ae476aee3f53cce on 01org:master.


[findings] findings from multi-progress

Here's a few gotchas I've run into so far -- will keep updating this issue as I run into things ;) I'm happy to tackle some of them as time allows, but we can start with talking about them :)

The progress/download bars work just fine without having called start() - they will just be default indicatif styling (ugly). Could be easy for a user to miss and not understand why the styling isn't working.

Some weird UI behavior that I haven't figured out yet (the only "Bitcoin Core" is the title of the multi_progress:

This is a simple multi-progress with download + spinner + spinner being added sequentially, with start and stop being called on them before creating the next bar.

EDIT: This seems to have something to do with the stop() method on either the progressbar or multibar -- you can see in the image in pt. 5 that when I don't call stop() on the multi_progress that this duplicate doesn't happen.

Would be better if with_ methods returned &mut Self, otherwise there's move issues in closures.

There should be a get_theme() method to return the current theme. I'm trying to "swap" the template of a progress bar from a spinner ("Preparing to download...") to a download bar ("Downloading...") and it's proving very difficult. "Preparing to download" is retrieving headers and getting the total size, and "Downloading" is when the size is known and the progress bar can start.

What I'd like to ideally have is a spinner while preparing, and switch to download once I know the size.
Here's a little example from the "download" step in the image from pt. 2:
let multi = cliclack::multi_progress("Bitcoin Core");
let dl = multi.add(progress_bar(100).with_spinner_template());
dl.bar().set_message("Download starting...");

let mut total_size = 0;
let mut progress = 0;

let bitcoin_core_archive = download_file(
    &bitcoin_url,
    &ctx.tmp_dir,
    |size| {
        total_size = size.clone();
        // `dl` MOVED HERE
        let dl = dl.with_download_template();
        dl.bar().set_message("Downloading Bitcoin Core...");
        dl.bar().set_length(total_size);
    },
    |chunk, _| {
        // `dl` has been moved...
        dl.bar().inc(chunk);
        progress += chunk;
    },
)
.await?;

dl.stop(format!(
    " {} {}",
    style("✔").green(),
    "Download Bitcoin Core"
));

If one forgets to call stop() on the multi_progress then the grouping fails:

Note that until multi.stop() is called, this formatting is applied (i.e. it's very obvious during longer steps like downloading).

I seem to be getting extra padding before the outro()? I didn't see this in your screenshot :thinking:

@cylewitruk Progress bars is a rabbit hole.
Alright,
pt.2: It's fixable, I can fix it.
pt.5: cliclack is literally a printer, if you forget to notify that the multi-bar is stopped, it doesn't become stopped. As an option, I have to implement a mediator pattern where the progress bars hold the reference to the multi-bar and they notify they're finished.
pt.6: the padding is noticeable if the multi-progress block is at the end of the list before the outro. There is always a gap to keep spacing before the next element.

@cylewitruk Progress bars is a rabbit hole.

Haha 🐰 It's a little fun, admit it 😉 They look pretty cool, anyway :)

pt.2: It's fixable, I can fix it.

Ok!

pt.5: cliclack is literally a printer, if you forget to notify that the multi-bar is stopped, it doesn't become stopped. As an option, I have to implement a mediator pattern where the progress bars hold the reference to the multi-bar and they notify they're finished.

I had this working in my PR branch, guessing I did something similar. Afk right now but iirc I had logic to print the left bar based on the bar's position in the multibar, so only the last one ever printed with the S_BAR_END which removed the flickering. Let me know if you want me to come with a suggestion-PR?

pt.6: the padding is noticeable if the multi-progress block is at the end of the list before the outro. There is always a gap to keep spacing before the next element.

Aha, okay -- in this particular case it feels a little weird visually to me - what do you think?

@cylewitruk

I had logic to print the left bar based on the bar's position in the multibar
I have the same logic.

What I'd like to ideally have is a spinner while preparing, and switch to download once I know the size.
Keep it simple: run the spinner, wait, show awaiting is done, and at the next step show a progress bar.

There should be a get_theme() method to return the current theme.
When you implement your theme it inherits ALL the methods of the default theme, you can retrieve any string from those methods.

For now, use 0.2.1 with fixes.

@cylewitruk and I still find it super buggy... More fixes to come

Yeah there's still some issues, but not all that bad imo -- stuff I'm putting in this ticket are just findings/thoughts, nothing that is a blocker so far, so no stress :) Feel free to tack-on things you find here as well so the info is consolidated -- I figure we can create issues from things here that should be changed/fixed instead of issue-spamming.

MultiProgress is private, so it can't be passed around (nice to be able to do when isolating different steps to smaller methods).

What I'd like to ideally have is a spinner while preparing, and switch to download once I know the size.

@cylewitruk Check the version 0.2.2, I added a clear() method, so you can now add the spinner, clear it, and continue with a progress bar!
Warning: I also had to stop exposing indicatif via the bar() method but it made rendering easier to implement.

What I'd like to ideally have is a spinner while preparing, and switch to download once I know the size.

@cylewitruk Check the version 0.2.2, I added a clear() method, so you can now add the spinner, clear it, and continue with a progress bar!

Great! Thanks :)

Warning: I also had to stop exposing indicatif via the bar() method but it made rendering easier to implement.

Hm, how do I set_message(), set_length() and inc() now?

P.S.: MultiProgress is public: docs.rs/cliclack/latest/cliclack/struct.MultiProgress.html

It worked now after a cargo update :+1:  (I'm using github directly atm)

Hm, how do I set_message(), set_length() and inc() now?

I have to add it to API :) Honestly, indicatif's finish_and_clear and other similar methods are out of control, corner cases only.

I have to add it to API :)

Hehe okay, wasn't sure if I was missing something :sweat_smile:

Honestly, indicatif's finish_and_clear and other similar methods are out of control, corner cases only.

A lot of the stuff in indicatif is slightly questionable imo :rofl:

@cylewitruk Check the main branch

@cylewitruk Check the main branch

@fadeevab theere we go, it builds again! :dancers: :champagne:  thanks :)

@cylewitruk Woow, noooice :)

Hmm... we've got something weird in async contexts... this is reading from a stream in a loop like:
while let Some(log) = log_stream.next().await { ... }

Screencast from 2024-04-04 21:58:13.webm
I'll try to make an MRE and see if I can't narrow it down...

Just so I don't forget, there's an extra space after the spinner multibar:

@cylewitruk To be sure I understand: do you mean an empty line under the cyan end bar?

@cylewitruk To be sure I understand: do you mean an empty line under the cyan end bar?

No, alignment of

@cylewitruk I can suggest redefining the theme: format_progress_start, it sets the 2 spaces.

@cylewitruk FYI, the patch is simple, however, 2 spaces look good as well. PR https://github.com/fadeevab/cliclack/pull/34

@cylewitruk Let's close this issue, everything seems fixed now. Let's start a new discussion if it's indeed necessary.


1115/min max validation

Issue

Closes #1115

Description
Please include a summary of the change and which issue(s) is addressed. Please also include relevant motivation and context. List any dependencies that are required for this change.
In Detroit, in New Listing --> Add Unit Group, you were previously able to successfully add values in Min form fields larger than in their corresponding Max fields without error. Now all of the following have custom errors, when min is larger than max:

Affordable Unit Group Quantity <---> Unit Group Vacancies
Occupancy
Square Footage
Floor
Bathrooms

How Can This Be Tested/Reviewed?

Entry Point: http://localhost:3001/listings/add

Go to New Listing in Detroit partners
Click the Add Unit Group link to open the Add Unit Group Modal
Choose Min values larger than their corresponding Max values
Press Save & Exit - you will see errors under all fields:

Provide instructions so we can review.
Describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration.
Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[x] I have reviewed the changes in a desktop view
[x] I have reviewed the changes in a mobile view
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[ ] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes
[ ] Any dependent changes have been merged and published in downstream modules
[x] I have assigned reviewers
[ ] I have run yarn generate:client and/or created a migration if I made backend changes that require them
[ ] I have exported any new pieces added to ui-components
[x] My commit message(s) is/are polished, and any breaking changes are indicated in the message and are well-described
[ ] Commits made across packages purposefully have the same commit message/version change, else are separated into different commits

Reviewer Notes:
Steps to review a PR:

Read and understand the issue
Review the code itself from a style point of view
Pull the changes down locally and test that the acceptance criteria is met
Also review the acceptance criteria on the Netlify deploy preview (noting that these do not yet include any backend changes made in the PR)
Either explicitly ask a clarifying question, request changes, or approve the PR if there are small remaining changes but the PR is otherwise good to go

On Merge:
If you have one commit and message, squash. If you need each message to be applied, rebase and merge.

#1163 Connecting this issue here - which addresses the failed checks here.


Support Parsl special options: parsl_resource_specification, stdout, stderr, and walltime

What new feature would you like to see?
As shown here, we need to be able to support Parsl "special options" with @job decorators: namely, parsl_resource_specification, stdout, stderr, and walltime.
Originally mentioned in https://github.com/Quantum-Accelerators/quacc/discussions/1881.

Comment from Parsl team:
YB: Here's how I see it from the HTEX perspective. An app behaves mostly like a python function, but with some additional parsl behavior (like walltime limits, parsl_resource_spec... ). If the issue is that you do not want to/can't  modify a user supplied function, you can always wrap your function with a wrapper that takes the parsl_resource_specification and internally calls the user supplied function.
AR: Yeah, the issue is that it would not be practical or feasible to modify the underlying functions. Interesting idea about the wrapper. That seems like a potentially viable path forward, but my concern would be that this would then mess up things with monitoring (e.g. the function name would be the wrapped function name and such). I guess that might be solvable with a @functools.wraps kind of thing?

I gave a quick shot at this, with partial success...
# In quacc.wflow_tools.job
@wraps(_func)
def wrapper(*f_args, parsl_resource_specification=None, **f_kwargs):
    f_kwargs.pop("stdout", None)
    f_kwargs.pop("stderr", None)
    f_kwargs.pop("walltime", None)
    f_kwargs.pop("parsl_resource_specification")
    return _func(*f_args, **f_kwargs)

Thinking that I could "trick" parsl into thinking that the keywords were passed without having to pass them to the actual job: this does not work. It seems that it does not matter how smart I try to be about it, keywords have to be passed to the actual jobs... For example for espresso recipes they would end up in **calc_kwargs which is not ideal... For recipe without any kwargs this would just crash.

Following yesterday's deleted comment, if you do the following change in the job decorator you get what you want. There is no additional assumptions about the parameter that _func takes. The only problem is what you already reported above: what is being launched is function job.<locals>.wrapper which can be problematic for some situations (monitoring...), doing wrapper.__name__ = _func.__name__ does not not seem to have any impact.

elif SETTINGS.WORKFLOW_ENGINE == "parsl":
    from parsl import python_app
    stdout = kwargs.pop("stdout", None)
    stderr = kwargs.pop("stderr", None)
    walltime = kwargs.pop("walltime", None)
    parsl_resource_specification = kwargs.pop("parsl_resource_specification", None)
    def wrapper(
        *f_args,
        stdout=stdout,
        stderr=stderr,
        walltime=walltime,
        parsl_resource_specification=parsl_resource_specification,
        **f_kwargs,
    ):
        return _func(*f_args, **f_kwargs)
    return python_app(wrapper, **kwargs)

config = Config(
    executors=[
        HighThroughputExecutor(
            label="htex_local",
            cores_per_worker=1,
            max_workers=1,
            provider=LocalProvider(
                channel=LocalChannel(),
                init_blocks=1,
                max_blocks=1,
            ),
        )
    ],
)

resource_spec = {
    "num_nodes": 2,
    "ranks_per_node": 2,
    "num_ranks": 4,
}

parsl.load(config)

@job(parsl_resource_specification=resource_spec)
def test_mpi_apps():
    from os import environ

    return environ.get("PARSL_RANKS_PER_NODE")

future = test_mpi_apps()

print(future.result())

This should print 2

Yup, that makes perfect sense to me. It's possible that @functools.wraps around the wrapper could fix the function metadata (should probably be used either way).
Feel free to open a PR if you would like. Otherwise, I appreciate your comment here and will get to it one day. :)


KIP-421: Support for resolving externalized secrets in AbstractConfig

The new constructor is similar to an existing constructor, with a flag enableAutoResolution to enable/disable automatic resolution of indirect variable. The originals configurations will contain both the config provider configs as well as configuration properties. The constructor will first instantiate the ConfigProviders using the config provider configs, then it will find all variables in the values of the originals configurations, attempt to resolve the variables using the named ConfigProviders, and then do the normal parsing and validation of the configurations. If ConfigProvider is not provided in the originals, the constructor will skip the variable substitution step and will simply validate and parse the supplied configuration.
Committer Checklist (excluded from commit message)

[x] Verify design and implementation
[x] Verify test coverage and CI build status
[x] Verify documentation (including upgrade notes)

Reviewer:
@rhauch @rajinisivaram Please could you'll review this PR.

@tadsul Can you fix the checkstyle violation in the build, please?

retest this please


Davide segullo/fix build

:tada: This PR is included in version 1.0.0 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


fix(module:button): fix add class ant-btn-icon-only(#7631)

PR Checklist
Please check if your PR fulfills the following requirements:

[x] The commit message follows our guidelines: https://github.com/NG-ZORRO/ng-zorro-antd/blob/master/CONTRIBUTING.md#commit
[x] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features)

PR Type
What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: 7631
What is the new behavior?
add class ant-btn-icon-only to button
Does this PR introduce a breaking change?
[ ] Yes
[x] No

any plan to merge this ?

The code looks good to be. But a unit test would be good. :) @withOutUndo could you please add one?

add some tests for this

when would this be merged?

@simplejason This could be merged in the next patch version.

Dear developers, please merge this already 🙏
Apps that dynamically load nz icons have been broken for a long while because of this. I've been waiting months for a fix so I can upgrade to the new version.


Add recordset summary to recordset app

This is tied to the paging feature #271, should be implemented when we have paging.

Show total number of records

Use number of records format: 'aaa-bbb of nnn items'


"exact" for "Redirect from"

Currently, it seems like the only way to redirect from the root to a particular sub-page is:
<Route exact path="/" render={() => <Redirect to="/dashboard" />} />

It would be a handy shortcut to be able to write (within a <Switch>, of course):
<Redirect from="/" exact to="/dashboard" />

It's possible this already works, but isn't documented.

See https://reacttraining.com/react-router/core/api/Switch/children-node and https://github.com/ReactTraining/react-router/issues/4791#issuecomment-287936977

Would you accept a docs PR to note that exact works in a Redirect nested in a Switch? The comment you link mentions it's unsupported that any component works, but since Redirect is officially supported it make might sense to document that option. Just let me know!

It could possibly be part of the <Switch children> docs, but it shouldn't be included in the <Redirect> docs.

Ah, I was going to finally do this, but you beat me to it! https://github.com/ReactTraining/react-router/pull/4849
🎉  Thank you!

The "to" and "from" seems like a neat approach that would allow an easier oversight.


feat(pie-card-container): DSW-1306 validate the padding prop

Describe your changes (can list changeset entries if preferable)

after raising this PR , I realized we could validate the padding prop in a simpler way than introducing a specific case to the validator.

Author Checklist (complete before requesting a review)

[ ] I have performed a self-review of my code
[ ] If it is a core feature, I have added thorough tests
[ ] If it is a PIE Docs change, I have reviewed the Docs site preview
[ ] If it is a component change, I have reviewed the Storybook preview
[ ] If there are visual test updates, I have reviewed them properly before approving

Reviewer checklists (complete before approving)
Reviewer 1

[ ] If it is a PIE Docs change, I have reviewed the PR preview
[ ] If there are visual test updates, I have reviewed them

Reviewer 2

[ ] If it is a PIE Docs change, I have reviewed the PR preview
[ ] If there are visual test updates, I have reviewed them

Nice one @raoufswe 🥇


[$500] HIGH: Remove @expensify.sms from SMS mentions

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 1.4.21-1
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
Action Performed:

Navigate to staging.new.expensify.com
Go to a #Room that has several members (
Enter "@" in the compose box
Select phone number member

Expected Result:
Phone number should be displayed in chat composer correctly
Actual Result:
Phone number member is displayed with suffix @expensify.sms
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/93399543/b2dd7d05-25cf-4ef8-b4a9-cab5ea78aad6
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0178aad4878bea5bbe
Upwork Job ID:<PHONE_NUMBER>961524736
Last Price Increase: 2024-01-17
Automatic offers:

c3024 | Reviewer | 28118032
FitseTLT | Contributor | 28118033

Proposal
Please re-state the problem that we are trying to solve in this issue.
Number contacts is displayed with suffix @expensify.sms
What is the root cause of that problem?
We are pushing the login without removing the suffix here
https://github.com/Expensify/App/blob/27a51a44e744f66956687550768e0037e6612923/src/pages/home/report/ReportActionCompose/SuggestionMention.js#L166-L167
What changes do you think we should make in order to solve the problem?

                    login: detail.login?.replace('@expensify.sms','')

What alternative solutions did you explore? (Optional)

Proposal
Please re-state the problem that we are trying to solve in this issue.
Phone number should be displayed in chat composer correctly
What is the root cause of that problem?
We get login when select the phone number in the mention list here
https://github.com/Expensify/App/blob/3826e5dd01cde3ae14d2f258a05a9514fa11e158/src/pages/home/report/ReportActionCompose/SuggestionMention.js#L78
What changes do you think we should make in order to solve the problem?
We should use LocalePhoneNumber.formatPhoneNumber() to remove sms domain with phone number
What alternative solutions did you explore? (Optional)
N/A

Proposal
Please re-state the problem that we are trying to solve in this issue.
Selection of a user mention with a phone number will show<EMAIL_ADDRESS>What is the root cause of that problem?
When we select a user mention, we will take their raw login.
https://github.com/Expensify/App/blob/d4c3e63956e9c14838592172c7e0e8596b0814cf/src/pages/home/report/ReportActionCompose/SuggestionMention.js#L78
The mention object is constructed here:
https://github.com/Expensify/App/blob/d4c3e63956e9c14838592172c7e0e8596b0814cf/src/pages/home/report/ReportActionCompose/SuggestionMention.js#L163-L166
and the login property is coming from the personal detail object. If it's a phone number, the<EMAIL_ADDRESS>suffix will be present.
Previously in this PR, we already removed the<EMAIL_ADDRESS>suffix for alternateText property and we previously used alternateText when selecting a mention.

What changes do you think we should make in order to solve the problem?

Take the mention object alternateText back.
<EMAIL_ADDRESS>

Proposal
Please re-state the problem that we are trying to solve in this issue.
Mention - Number contacts is displayed with suffix @expensify.sms
What is the root cause of that problem?
<TNodeChildrenRenderer tnode={props.tnode} /> render props.tnode.init.textNode.data that show full +XXXXXX'@expensify'
What changes do you think we should make in order to solve the problem?
Changeprops.tnode.init.textNode.data by splitting at@expensify and acces first element
props.tnode.init.textNode.data = displayNameOrLogin.split('@expensify')[0]

What alternative solutions did you explore? (Optional)

@c3024 any thoughts on the proposals above?

@FitseTLT @namhihi237 can you post the screenshot/video of how the sent mention messages appear? Are they pressable and does pressing on them show the details page of the mentioned user?

I think we should hold this issue utils here https://github.com/Expensify/App/issues/18729 to solve

Hello @c3024 . Mine have the screenshot video you requested. Here.
https://github.com/Expensify/App/issues/33902#issuecomment-1876676332

I think we should hold this issue utils here https://github.com/Expensify/App/issues/18729 to solve

@c3024 does that sound good to you? If not, can you please review the proposals above? Thanks!

I'll check today

@c3024 any updates? Thanks!

Just got a clarification from here.
@aim-salam can you see if you can remove the @expensify.sms in the composer as well and update your proposal accordingly?

Proposal
Please re-state the problem that we are trying to solve in this issue.
Mention - Number contacts is displayed with suffix @expensify.sms
What is the root cause of that problem?
In SuggestionMention.js,
after phone number mentioned, updateComment( ) show mentionCode as<EMAIL_ADDRESS>https://github.com/Expensify/App/blob/744e35c85157ecef9cdfab81a0ab56891dc507f9/src/pages/home/report/ReportActionCompose/SuggestionMention.js#L81
What changes do you think we should make in order to solve the problem?
Remove mentionCode's @expensify.sms into '' whenever phone number mentioned.
//const into let
+            let mentionCode = mentionObject.text === CONST.AUTO_COMPLETE_SUGGESTER.HERE_TEXT ? CONST.AUTO_COMPLETE_SUGGESTER.HERE_TEXT<EMAIL_ADDRESS>            const commentAfterMention = value.slice(suggestionValues.atSignIndex + suggestionValues.mentionPrefix.length + 1);

+            mentionCode = mentionCode.replace('@expensify.sms','')

But when we send the chat, the suggested number didn't mention.

In reportactioncompose.js, we need to add back<EMAIL_ADDRESS>into phone number before we submit.
https://github.com/Expensify/App/blob/744e35c85157ecef9cdfab81a0ab56891dc507f9/src/pages/home/report/ReportActionCompose/ReportActionCompose.js#L279-L293
            const newComment = composerRef.current.prepareCommentAndResetComposer();
            if (!newComment) {
                return;
            }

+           const words = newComment.split(' ')
+           const updatedWords = words.map((word) => {
+                    if (word.charAt(0) === '@' && word.charAt(1) === '+') {
+                        return word +<EMAIL_ADDRESS>+                   } else {
+                       return word;
+                    }
+                })
+             const updatedComment = updatedWords.join(' ');

+            onSubmit(updatedComment);

https://github.com/Expensify/App/assets/142758714/1696c718-6b95-4b9e-b93d-dc450b23fe88
What alternative solutions did you explore? (Optional)

@c3024 I misunderstood here;

I got confused by the video shown.
I proposed a new proposal https://github.com/Expensify/App/issues/33902#issuecomment-1890335529

@c3024 I misunderstood here;
I got confused by the video shown.
I proposed a new proposal https://github.com/Expensify/App/issues/33902#issuecomment-1890335529

Updated proposal

@c3024 any thoughts on the updated proposals above? Thanks!

I will update on the updated proposals today.

@puneetlath
We are removing the @expensify.sms portion of the mentions in this issue.
We generally use formatPhoneNumber which uses parsePhoneNumber to remove the SMS domain. But for user mentions this is not feasible because this parsePhoneNumber breaks the phone number into parts separated by spaces like this (and country codes of same country numbers are also differently parsed based on IP with this code here)

and mention strings cannot have spaces in between for proper parsing with ExpensiMark of these into mentions.
So we need to use the Str.removeSMSDomain to remove the sms domain and the result will be like this for mentions

and they look like these in composer

Can you please confirm if we can go ahead with parsing like these two screenshots above with Str.removeSMSDomain?

@puneetlath bump on the comment above ^^^ thanks!

That sounds good to me.

Proposal here by @FitseTLT looks good to me.
🎀 👀 🎀 C+ Reviewed

@jasperhuangg @c3024  This issue needs a change in our common library. So should I raise a PR and tag u guys on common lib repo??

Yes, a PR on expensify-common needs to be be raised and merged.
Then we bump the version in another PR along with other changes required on Expensify/App.
You can check a recent PR on expensify-common.

Will create the PR on expensify-common in 2 days 👍

Sounds good — thanks @FitseTLT!

@FitseTLT any update on the PR? Thanks!

@FitseTLT bump ^^^ thanks!

Thanks @FitseTLT! @c3024 the PR is ready for review 👍
@jasperhuangg — bump on my comment here. Thanks!

@CortneyOfstad This looks like a #vip-vsb type of issue,j since it relates to mentions (which are not related to core expense features).

Awesome — thanks @jasperhuangg!

@c3024 any update on the PR review? Thanks!

Commented on the PR on expensify-common. Could you check it? @FitseTLT

Looking back at this issue, I think I might have misread what the issue was.
We need the @expensify.sms domain in the compose box when we send a mention since that's the only way we can identify a user, that is their login.
I thought we were talking about the domain existing in the report action after it is sent, but that's an entirely different issue. I believe we can just close this issue out without taking any action.

@jasperhuangg I had the same question if we should remove it in composer as well so I asked here and @puneetlath suggested here that we better remove it in composer as well.
If we would like to remove it only in the report action also, we can address it in this same issue.

Ah got it, that's my bad, I was under the impression that the expensify.sms domain was necessary in the composer. In that case let's keep this open, thanks for clarifying.
I spun up a revert of the linked PR since after testing it it didn't work.

Created new PR with the test steps and also test result and tagged the reviewers 👍

Awesome — thanks @FitseTLT!

Woohoo, awesome progress.  What is the ETA for merge, and what specifically remains?  Who is doing it, when?

On https://github.com/Expensify/expensify-common/pull/647 we need some backend job to be done in parallel to its change so @jasperhuangg is on it. As soon as it is finished, the change we need to make here won't take time 👍

I'm looking into the back-end fix today, I'll ping here when it's done. Looking to get the fix out for review today.

Awesome — thanks @jasperhuangg!

@jasperhuangg is there an update on this? Do you think this would be completed by today, EOW, etc? Thanks!

@CortneyOfstad ah sorry, I had some local changes but some other priorities came up, I'll be sure to get the changes polished up and ready for review today.

@FitseTLT @CortneyOfstad responded with some feedback in the PR! https://github.com/Expensify/expensify-common/pull/647#issuecomment-1955248478

Looks to be good activity on the PR? Do you think that this is something that can be wrapped by EOD or by early next week @FitseTLT @jasperhuangg?

Bump @FitseTLT @jasperhuangg^^^ thanks!

Only left with @jasperhuangg final approval on the expensify-common PR 👍

@CortneyOfstad we merged the library fixes but need another App PR to wrap things up, which we should be able to get out early next week.

Awesome — thank you!

Not overdue per this comment here 👍

@FitseTLT any updates on that App PR to prevent rendering the expensify.sms domain with report actions?

I think I can make it ready today or tomorrow

@quinthar We just merged the expensify-common PR that updates Expensimark.
@FitseTLT just needs to open another PR that ensures we don't display SMS domains in the front-end.

Thanks @jasperhuangg!
@FitseTLT can you provide an update on when you can have the other PR opened by EOD?

Yep

@FitseTLT what's your ETA for a PR?

PR is up and on reviewing stage

Great! @c3024 ready for you!

Hi @garrettmknight! I am heading OoO until March 11th, so reassigned this to keep it moving until I am back. As of right now, the PR  from @FitseTLT is waiting for @c3024 to review. Thanks!

I've reviewed the PR and suggested some changes.

I am asking for a response and confirmation from @jasperhuangg on the PR

hi!  What exactly remains to be done, who's doing it, and what is the ETA?

I just responded to @FitseTLT's question on their PR. It's looking mostly good and we should be able to review and merge it soon.

We're waiting on @FitseTLT to fix some leftover review comments from @c3024

Did this happen?  What is the holdup; what is the new ETA?

@CortneyOfstad C+ review is done just waiting to be merged 👍

What is the next step, who is doing it, and when?  Are we waiting on @c3024 to merge?  When can that happen?

There were conflicts and they have been fixed by @FitseTLT. The changes look good. @jasperhuangg will likely merge the PR by today.

Merged!

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[x]  [@c3024] The PR that introduced the bug has been identified. Link to the PR: NA as this is a new feature.
[x]  [@c3024] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: NA
[x]  [@c3024] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: Not started because this is a new feature
[x]  [@c3024] Determine if we should create a regression test for this bug. Yes
[x]  [@c3024] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression test proposal
This test in Tests of PR #33902 covers multiple cases. It is a good regression test.

Go to any room and type @
Select a user with phone number login
Verify that the phone number inserted in the compose doesn't have the sms domain suffix @expensify.sms
Send message and verify that the phone number is correct mention doesn't have the suffix and hover hover it and check the tooltip doesn't have it too
Click on it and verify the profile page displays the phone number without the suffix
On the system message click on invite the user to the room and on the message that indicates the user is invited check the tooltip displays without the suffix and click on it and verify the details page displays it without the suffix

Payment Summary

@FitseTLT — paid $500 via Upwork
@c3024 — pai $500 via Upwork

Regression Test
https://github.com/Expensify/Expensify/issues/380809
Done and done! Thanks all!


Add NetBeans support to IDE project generator.

NetBeans has really, really good support for dealing with Makefiles and code completion (including handling various preprocessor define settings) after parsing the make output. It also has really good support for setting up an ARM GCC cross-compiling toolchain.
Unfortunately, the Makefiles currently being generated by mbed-cli for mbed OS 5 projects aren't understood by NetBeans, it can't seem to find the source files.
I only see errors like:

$ make -f projectfiles/gcc_arm_NRF51_DK/Makefile
make[1]: *** No rule to make target 'mbed-os/rtos/rtx/TARGET_CORTEX_M/TARGET_M0/TOOLCHAIN_GCC/SVC_Table.o', needed by 'Unnamed_Project.elf'.  Stop.
projectfiles/gcc_arm_NRF51_DK/Makefile:21: recipe for target 'all' failed
make: *** [all] Error 2

The old exporter from the mbed Classic online IDE could generate Makefiles that could be imported into NetBeans seamlessly.
One difference seems to be that the Makefiles in Classic were generated into the root folder of the source code, where as the Makefiles in mbed-cli are generated under the projectfiles folder and somehow NetBeans can't pick up on how the make command is moving around the directory tree to get all of its sources.
I've attached the Makefile, nbproject/project.xml, and nbproject/configurations.xml for an mbed Classic app that I imported seamlessly into NetBeans, in case someone would be willing to take up the challenge of adding support for this for mbed OS 5 projects.
(That person could even be me, but I'd need some pointers on how the project generator gets called from mbed CLI.)
project-files.zip

@sarahmarshy @0xc0170 @theotherjimmy

FYI: I just tried an mbed export -i gcc_arm -m K64F within the mbed-os-example-client project with mbed-os checked out to https://github.com/ARMmbed/mbed-os/pull/2245 and imported the resulting project into netbeans. The compile worked.

Ok, it works for the most part. I'm still testing.
Refactor requests:

If the export is for gcc_arm, it would be nice if the GCC_BIN setting in the Makefile were automatically set to the value of GCC_ARM_PATH in the .mbed file, if that value is already set for a project.
Also, it would be nice if the PROJECT name in the Makefile could be settable by adding something like mbed export -n <name>.

There seemed to be a linker error to one of the functions I was calling from mbedtls in one of my projects, but I'll have to doublecheck that. It compiled file in Netbeans, but then did one of these numbers on me:
Test.cpp:(.text._ZL7keyMeDov+0x46): undefined reference to 'mbedtls_pkcs5_pbkdf2_hmac'

Nevermind on that last point, I forgot to add -DMBED_PKCS5_C to the command line.

cc @theotherjimmy

If the export is for gcc_arm, it would be nice if the GCC_BIN setting in the Makefile were automatically set to the value of GCC_ARM_PATH in the .mbed file, if that value is already set for a project.

This should be happening. See here: https://github.com/theotherjimmy/mbed/blob/1fb449cdd3ff7af47f684dd3a51586e011df6b7c/tools/export/makefile/init.py#L62-L65 The toolchain is being queried for it's compiler command. It looks like the problem lies with the toolchain GCC, as it does not set it's cc, cppc, etc. members to match the exact location of gcc in the FS.
If you have armc5 installed, you could export to make_armc5/make_iar (with the above PR) and see that it uses the toolchain paths (That would also be pretty cool: a NetBeans project compiling with armc5 or IAR).

Also, it would be nice if the PROJECT name in the Makefile could be settable by adding something like mbed export -n .

This seems reasonable, except that mbed export -n is already taken. I suggest --name instead. You're welcome to submit a counter-proposal for the flag, I'm not attached to --name.

@nuket are you happy with the provided answer above? Also do you think this conversation can be taken over to https://github.com/ARMmbed/mbed-os instead?

@screamerbg Yes, the export system has been working fine for me. I think this ticket could be closed.

@screamerbg I just noticed that this issue is still open. Would you like to close it now?


No matching export

New install on fedora 40.
Blank project (npm init)
On unning npm start get:
Error: Build failed with 2 errors:
node_modules/@revideo/core/lib/types/Color.js:1:9: ERROR: No matching export in "node_modules/chroma-js/index.js" for import "Color"
node_modules/@revideo/core/lib/types/Color.js:1:16: ERROR: No matching export in "node_modules/chroma-js/index.js" for import "mix"

hey, thanks for reporting this! I've already responded to this in detail in #228, feel free to check out my response there.
TLDR:

this is caused by a newer version of chroma-js. To fix it, downgrade chroma-js to 2.4.x.
our newest release (will release today) will include a fix (pinned version of chroma-js)

Thanks for reporting! We just released 0.5.3 which should fix this. Feel free to reopen in case you're still facing issues related to chroma-js after upgrading!


Refactoring getting-started docs and automations

This PR refactors both getting started documentation and node-join automation fon KinD.
In detail adds ability to set differnent TCP kubeconfig path, other than /tmp/kubeconfig during worker node join.

@prometherion I resolved conflicts on the getting-started-guide.md and refactored it according to new Kine integration and Make targets to easy the quickstart in case of standard-etcd installation. PTAL :-)


When I use it on SD webui, I only got lots of pictures, not videos

Does original AnimateDiff model works for you on SD webui?


cannot unpack non-iterable NoneType object on loading camemBERT

🐛 Bug
When I try to load the model on google colab :
import torch camembert = torch.hub.load('pytorch/fairseq','camembert')
I get this error on loading :
Using cache found in /root/.cache/torch/hub/pytorch_fairseq_main
TypeError: cannot unpack non-iterable NoneType object

Anyone knows how to troubleshoot ?
Before having this error, I had two others while following camemBERT's documentation : missing hydra-core and missing bitarray. Fixed them by installing the correct packages.

I got the same error after installing hydra-core and bitarray. Restarting the python kernel revealed that other packages were missing as well. After installing the new pip package, and rerunning, I got the TypeError: cannot unpack non-iterable NoneType object again. I then had to restart the kernel again. After a few cycles of installing a missing dependency and restarting the kernel, the problem resolved.

As @ChristianHinge  said, please try restarting python kernel and see if there is a different output.

1835  2023-05-28 18:04:26 root pip install tensorboardX
1889  2023-05-28 18:39:47 root pip install soundfile
1902  2023-05-28 18:49:17 root pip install editdistance
1904  2023-05-28 18:49:50 root pip install hydra
1906  2023-05-28 18:50:45 root pip install hydra-core


Disable selection on resize

Closes #2034
https://user-images.githubusercontent.com/3683420/185420859-bb4d74e6-4f0f-40f0-be8c-0a5dc60b904d.mov

@sroy3 a couple of UI tests seem broken, can you take a look, please?

@sroy3 a couple of UI tests seem broken, can you take a look, please?

Wrapped the table inside a div, not sure why it was doing this, but it's fixed.


Confusing which Xcode I'm using with a nightly toolchain

Now that Xcode 16 Beta is out I have two Xcodes installed on my machine. I have also downloaded the nightly build of the 6.0 Swift toolchain. Sometimes I want to use it with Xcode 15, sometimes with Xcode 16 Beta. When I use the command to select a Swift toolchain it's confusing which Xcode it's actually using. It would be nice if I could select that separately from the toolchain selection. At the end of they day, they are different environment variables on Mac, DEVELOPER_DIR and TOOLCHAINS.

When you select an Xcode installation you are selecting the toolchain from that Xcode install. If you select a toolchain install it isn't using your Xcode install at all, outside of one thing, the xctest library. I'm not certain offhand which Xcode version of xctest it would be using. Maybe @matthewbastien who wrote the toolchain select can answer that.
If you are using swift-testing selecting a toolchain install should mean you aren't using an Xcode install at all. In theory you shouldn't even need Xcode installed. I haven't tested that though.
We are not using the DEVELOPER_DIR or TOOLCHAINS environment variables.

Just for clarity, swift-testing is included in the Xcode 16 Beta. This means that if you are using Xcode and then switch to a toolchain install you may need to explicitly specify swift-testing as a dependency.

Just for clarity, swift-testing is included in the Xcode 16 Beta. This means that if you are using Xcode and then switch to a toolchain install you may need to explicitly specify swift-testing as a dependency.

You'd need to do the same if your package is compiled on Linux. I am assuming this will not be necessary once Swift 6 has a proper release though.

We are not using the DEVELOPER_DIR or TOOLCHAINS environment variables.

I wonder if that's what we should be doing on macOS. That would allow us to be consistent with the command line experience.
Though as it is, if you swift build from the terminal inside VSCode you actually get different settings right now.

Though as it is, if you swift build from the terminal inside VSCode you actually get different settings right now.

Yeah not ideal
Even if we used DEVELOPER_DIR we'd have this situation. We cannot affect the environment variables for the whole of VSCode, only the processes we start eg swift build etc. The terminals are setup by VSCode not the extension. There is a thing called a terminal profile, which can be used to setup a custom environment See #914 , I don't know if that affects all terminals, or add a new type of terminal.

My thoughts so far are that there are two scenarios here when the user selects a toolchain:

An Xcode toolchain is selected in which case we should be setting DEVELOPER_DIR to the Xcode path.
A public/nightly toolchain is selected which is a bit trickier. If there are multiple Xcodes installed on the system then we should ask the user which Xcode they want to select - like the Swift: Select Xcode Developer Dir... command we used to have before the toolchain selection obsoleted it.

It really boils down to the fact that we should be setting DEVELOPER_DIR if there are multiple Xcodes installed on the system. The TOOLCHAINS environment variable is also straightforward to set alongside our swift.path configuration which I can experiment with. I'm working on a PR to push up that folks can try out.

A twist in all this is that the CommandLineTools doesn't support the TOOLCHAINS env var. That makes me much less enthusiastic about relying on it.
For cross platform, I can see a scenario where the user does an xcode-select --install to get a minimal development environment and then installs the latest Swift toolchain for things like Swift Embedded. But we'll need to actually set the PATH to the toolchain to pick that up properly.

Verified in<PHONE_NUMBER>b96db4842fd74fa6b7f56f7c987098


Issue with ":memory:" value

This key doesn't seem to work when I pass the value ":memory:". The code below works if I use any other name.
defmodule SqilteConn do
  def start do
    Exqlite.Sqlite3.open(":memory:")
  end
  def create_table(conn) do
    Exqlite.Sqlite3.execute(
      conn,
      "CREATE TABLE IF NOT EXISTS csv (
        id integer primary key, ....)
     ')
  end
end
{:ok, conn} = SqilteConn.start()
SqilteConn.create_table(conn,)

defmodule Repo do
  use Ecto.Repo, adapter: Ecto.Adapters.SQLite3, otp_app: :noop
end

defmodule Airport do
  use Ecto.schema
  ...
end

{:ok, pid} = Repo.start_link(database: ":memory:", default_chunk_size: 100)
Repo.all(Airport)

returns:
** (Exqlite.Error) no such table: csv

instead of [].

Running Repo.start_link(database: ":memory:", default_chunk_size: 100) will use the default pool size where each database connection in the pool will open its own in memory database.
If you want to use an in memory database, you'll need to crank the pool size down to 1. I don't recommend doing that right now for application code because you'll have to migrate / load a structure everytime the connection is opened.
When you use any other value for the database name, it's just using that value as the path to the database from the application directory.

OK., thanks! The in-memory use case was for comparison with Surrealdb, not for a production case.


Make private vars protected so we can extend classes

This is one way of fixing #1 by allowing users to extend the Item class and use that in creating their menus.

I didn't think of that. thanks.


[Error] error parsing regexp: invalid escape sequence: \1

When I try to use the 'Artifactory NuGet push' I ran in the error below (similar to the issue #5 ).
If I modify the Path to NuGet package(s) to publish from '$(Build.ArtifactStagingDirectory)/*.nupkg' to the complete name of the package (i.e. $(Build.ArtifactStagingDirectory)/Module.$(FullVersion).nupkg) everything is ok.
Here are the logs:
2018-09-07T10:14:26.2810656Z ##[section]Starting: Artifactory NuGet push
2018-09-07T10:14:26.2819160Z ==============================================================================
2018-09-07T10:14:26.2820002Z Task         : Artifactory Nuget
2018-09-07T10:14:26.2820679Z Description  : Restore and push NuGet packages from and to Artifactory while allowing to collect build-info. The collected build-info can be later published to Artifactory by the "Artifactory Publish Build Info" task.
2018-09-07T10:14:26.2821367Z Version      : 1.1.0
2018-09-07T10:14:26.2821728Z Author       : JFrog
2018-09-07T10:14:26.2822222Z Help         : More Information
2018-09-07T10:14:26.2822783Z ==============================================================================
2018-09-07T10:14:27.3066415Z Running jfrog-cli from D:\vsts\agents\1_work_jfrog\current\jfrog.exe.
2018-09-07T10:14:42.3783116Z JFrog CLI version: 1.19.1
2018-09-07T10:14:42.4310653Z Collecting environment variables...
2018-09-07T10:14:46.4534661Z [Info] Collecting environment variables...
2018-09-07T10:14:46.4601282Z [Info] Collected environment variables for Scripts/20180907.8.
2018-09-07T10:14:46.5471874Z [Error] error parsing regexp: invalid escape sequence: \1
2018-09-07T10:14:46.5473961Z {
2018-09-07T10:14:46.5474986Z [Error] Upload finished with errors. Please review the logs
2018-09-07T10:14:46.5475423Z   "status": "failure",
2018-09-07T10:14:46.5478208Z   "totals": {
2018-09-07T10:14:46.5478926Z     "success": 0,
2018-09-07T10:14:46.5479637Z     "failure": 0
2018-09-07T10:14:46.5480215Z   }
2018-09-07T10:14:46.5480859Z }
2018-09-07T10:14:46.5568056Z ##[error]Error: Command failed: D:\vsts\agents\1_work_jfrog\current\jfrog.exe rt u D:\vsts\agents\1_work\13\a*.nupkg repository/Scripts/20180907.8/Module/ --build-name="Scripts" --build-number="20180907.8" --url="http://artifactory.net/artifactory" --user=*** --password=***
2018-09-07T10:14:46.5572186Z Error: Command failed: D:\vsts\agents\1_work_jfrog\current\jfrog.exe rt u D:\vsts\agents\1_work\13\a*.nupkg repository/Scripts/20180907.8/Module/ --build-name="Scripts" --build-number="20180907.8" --url="http://artifactory.net/artifactory" --user=*** --password=***
2018-09-07T10:14:46.5734030Z ##[section]Finishing: Artifactory NuGet push
The backslashes of the Windows path on the server seems to not be correctly escaped.
Can you please check?

Thanks for reporting this issue, @neolutin.
We are looking into it.

I have indeed the same issue, even with the default value of **/*.nupkg. Hopefully the PR of @yahavi will be pushed and released soon, because this is kind of a show stopper for us...

@dnperfors,
We'd like to release a patch with the fix by the end of the week.

@neolutin and @dnperfors,
@yahavi's PR has been merged (thanks @yahavi!) and version 1.1.1 has been released.
Please let us know if the new release resolves the issue for you.

@eyalbe4 yes I can confirm that it is working for me, thank you.

Happy to hear this 😄

@eyalbe4 Ok for me too.
Thanks a lot


Support for SHA256 fingerprint

Would it be possible to support SHA256 fingerprints in the config? The latest Ubuntu LTS release defaults to SHA256 fingerprints.
I realize that I can list the MD5 fingerprint, but its annoying and I'd like to banish md5 from everything if possible.

Yes. I actually have a branch somewhere that does this. The code quickly got more complicated than I liked when I tried to seamlessly support both the legacy md5 format and the newer prefixed format. Knowing what I know now I can probably redo this reasonably easily. So, yes, I can likely add support for this with backwards compatibility for both older versions of openssh and existing installations of ssh-cert-authority that still use the md5 format.

I am also interested in this feature. In the meantime, what do you suggest as a work-around?

tl;dr The workaround is to use -E md5 and strip the MD5 prefix from the output.
I'll bring a little context to the picture here. There are three places where these fingerprints matter: the client (requester) configuration, signer configuration and the server configuration.
For the requester side you no longer are required to use a fingerprint. Instead you can specify the filename of your key using PublicKeyPath. I realize now that USAGE.rst is out of date as it only shows the filename option. I've patched up the file to recommend simply using the path option and that has been pushed.
For the signer, for some reason, I only support fingerprints and on the server, because we don't have the full public key, we only support fingerprints. In both cases the fingerprint must be legacy MD5. To get this fingerprint you can use the -E md5 option to most openssh commands. For example, you might:
$ ssh-keygen -l -f ~/.ssh/bvanzant-yubikey-pkcs11.pub -E md5 2048 MD5:77:f4:fe:59:f5:1b:94:39:cb:27:75:ad:f4:0e:1e:ed no comment (RSA)
The output of that command is still invalid from an ssh-cert-authority perspective. You need to strip the MD5: prefix from the fingerprint.

What is the status for this item? Still in the planning or superseded by something else?


KAFKA-7141: ConsumerGroupCommand should describe group assignment eve…

…n with no offsets  committed.
https://issues.apache.org/jira/browse/KAFKA-7141
Currently, if a consumer group never commits offsets, ConsumerGroupCommand cannot describe it at all even if the member assignment is valid. Instead, the tool should be able to describe the group information showing empty current_offset and LAG.
More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.
Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.
Committer Checklist (excluded from commit message)

[ ] Verify design and implementation
[ ] Verify test coverage and CI build status
[ ] Verify documentation (including upgrade notes)

@vahidhashemian Please review this patch. Thanks.

@vahidhashemian Thanks for the comments, please review again.
@hachikuji Please review.

LGTM, thanks for addressing my comment.

@hachikuji Could you take some time to review this patch? Thanks.

With Vahid's previous KIP, I think the intention is that the default behavior of --describe is the same as --describe --offsets, so I'm wondering if the behavior is actually reasonable. We also have --describe --members which can be used to see the members in the group.

Ok, that makes sense.

retest it please.

retest this please

Thanks @huxihx. I will merge in the morning after the build finishes.

retest this please

retest this please

FAILURE
9284 tests run, 5 skipped, 3 failed.
--none--

@huxihx The build failure on jdk10 seems deterministic. Can you look into it?

retest this please

@hachikuji Seems it hit a known Scala bug opened by Ijuma (https://github.com/scala/bug/issues/10418). I will take care of it.

retest this please

@huxihx Thanks for fixing the issue. Merged to trunk.


Cache is not saved.

I use this SDK in different requests, and, as i can see in my /tmp folder - phantomJs cache is removed every time, script finished. But how can i keep pages cached forever (like chrome and others)? All i found in internet - enable disk-cache and increase disk cache size, but how can i do something like this using this SDK?
Sorry for my bad english..

You can add additional PhantomJS options using the addOption method:
$client = Client::getInstance();
$client->getEngine()->addOption('--disk-cache=true');
$client->getEngine()->addOption('---max-disk-cache-size=[size]');

Documentation
Let me know if this solves your issue.

Yap, that's it, everything is fast now, THANKS!
Missed this in documentation =(


Fixed #24908: exclude wouldn't apply on inherited form fields.

If a ModelForm overrides a form field which would have been
automatically instanciated for a form field, then extending that model
form with this field in Meta.exclude should indeed exclude that field.
At this point, the test fails and the excluded field ends up in
base_fields.

@jpic Put fields = '__all__' on your modelform and your patch will break, might be enough to move the elif into an if (also drop the ALL_FIELDS for exclude, that is never used in the docs)

Thanks for that, and there might also be other similar cases for which this patch isn't sufficient for, ie. overridden field not in child modelform's Meta.fields which should be treated the same way.

buildbot, add to whitelist.

Actually, I don't think we want to change the current behavior like this. Please see the commits at the end of ticket #8620.

Hopefully there is no security issue there because save_instance gets passed exclude which is used by construct_instance, way to go B)
Anyway, raising an exception in this case just like we do when an item of exclude is not recognized as a model field might be a good idea, what do you think ?
At least, we could document this more explicitely, in which case a bugfix for the double readonly field rendering which was initialy reported would be useful.

Not sure that I agree, after all this is a field which is baked by a model
field and where the last modelform in the chain chooses to exclude it....
On Thu, Jun 4, 2015, 18:17 Tim Graham<EMAIL_ADDRESS>wrote:

Actually, I don't think we want to change the current behavior like this.
Please see the commits at the end of ticket #8620
https://code.djangoproject.com/ticket/8620.
—
Reply to this email directly or view it on GitHub
https://github.com/django/django/pull/4747#issuecomment-108979909.

My understand is that Meta is only meant to affect auto-generated fields. In the test of this PR, since url = forms.URLField() is explicitly defined in the parent class, the way to exclude it in the child class is url=None. @loic, your thoughts?

My understanding is that meta is for model backed fields, no matter if auto
generated or not. The big question now is if this is a bug in admin or
forms :p
On Fri, Jun 5, 2015, 02:54 Tim Graham<EMAIL_ADDRESS>wrote:

My understand is that Meta is only meant to affect auto-generated fields.
In the test of this PR, since url = forms.URLField() is explicitly
defined in the parent class, the way to exclude it in the child class is
url=None. @loic https://github.com/loic, your thoughts?
—
Reply to this email directly or view it on GitHub
https://github.com/django/django/pull/4747#issuecomment-109123706.

@timgraham while it makes sense to use url=None instead of exclude=['url'], wouldn't it be useful to raise an exception when a field in the exclude has been overridden just like we do for fileds which aren't in the model ?

Here's a possible argument in favor of the first option (breaking BC by making exclude work as naively expected) is: why would I add a field in exclude if I wanted it not excluded if I have a form field override ? I mean, if I have a non-model form field in a parent subclass which I wouldn't want to be exclude-able, wouldn't it name it differently than model fields in the first place ?
Hopes this makes sense ^^

Still don't know if we're going to merge this, but I've completed it for further reference at least.
Thanks @dgouldin for the heads up.

Form.Meta is only about generating/generated form fields (maybe with the exception of Meta.error_messages['__all__']). I'm not sure whether it's the best design (e.g. DRF's Serializer.Meta controls both declared and generated fields) but changing exclude would be a pretty serious change at this point. Also exclude being the complement fields we'd have an inconsistant API.
Doesn't field=None suffice to avoid the double readonly thing?

I'll try that however do you think we should leave this as is or at least
raise an exception like we do for exclude items which are not fields of the
models ? Sorry for repeating myself but a final statement here would be so
awesome !
Le 5 juin 2015 15:33, "Loïc Bistuer"<EMAIL_ADDRESS>a écrit :

Form.Meta is only about generating/generated form fields (maybe with the
exception of Meta.error_messages['all']). I'm not sure whether it's
the best design (e.g. DRF's Serializer.Meta controls both declared and
generated fields) but changing exclude would be a pretty serious change
at this point. Also exclude being the complement fields we'd have an
inconsistant API.
Doesn't field=None suffice to avoid the double readonly thing?
—
Reply to this email directly or view it on GitHub
https://github.com/django/django/pull/4747#issuecomment-109311956.

I'll support this usage in the admin but that's a bit of a shame, if it's
not a bug a feature: I'd like to know if there's any sane use case for
this, because I can't think of any. If we're going to do anything about
this then the work done to support this in the admin will be useless :) I
got checkmated hahahaha
Le 5 juin 2015 15:46, "James Pic"<EMAIL_ADDRESS>a écrit :

I'll try that however do you think we should leave this as is or at least
raise an exception like we do for exclude items which are not fields of the
models ? Sorry for repeating myself but a final statement here would be so
awesome !
Le 5 juin 2015 15:33, "Loïc Bistuer"<EMAIL_ADDRESS>a écrit :

Form.Meta is only about generating/generated form fields (maybe with the
exception of Meta.error_messages['all']). I'm not sure whether it's
the best design (e.g. DRF's Serializer.Meta controls both declared and
generated fields) but changing exclude would be a pretty serious change
at this point. Also exclude being the complement fields we'd have an
inconsistant API.
Doesn't field=None suffice to avoid the double readonly thing?
—
Reply to this email directly or view it on GitHub
https://github.com/django/django/pull/4747#issuecomment-109311956.

My bad, I have found the bug in the admin

My bad, there are two bugs in the admin to get that result, probably why It
confused me so much heh
Le 5 juin 2015 20:24, "James Pic"<EMAIL_ADDRESS>a écrit :

My bad, I have found the bug in the admin

Ok, let's avoid fixing this for now and let energy be spent by others on this later, closing this PR in favor of https://github.com/django/django/pull/4822


[Feature Request] Write short description here

Why/User Benefit/User Problem
Faster loading pages...already switch both server but still slow...im from sea region..even using vpn also still slow

I hope faster pages loading only

Download ahead of time as its multi threaded and use a vpn.
see #5  https://mangadex.org/thread/236921


Cannot access file: bin/StrainPro-rep

For StrainPro v0.9.2, it appears that when calling StrainPro-build, that executable requires  StrainPro-rep to be located at bin/StrainPro-rep relative to the user's current working directory. Thus the user is required to always call StrainPro-build from the base StrainPro directory.
Why not just provide instructions for adding /path/to/clone/of/strainpro/StrainPro/bin/ to the user's directory and remove this hard-coded path requirement for bin/StrainPro-rep in bin/StrainPro-build? That is what the PATH env variable is for, and it would make bin/StrainPro-build a lot more flexible.

@hsinnan75 I just wanted to ping you about the -dump /path/to/the/taxdump/ issue, as explained above. Any progress on fixing that issue?

Sorry, I forgot to fix this issue. I'll do it asap and let you know.

Hi, I just found a bug. I forgot to include "-dump " when StrainPro-build asks StrainPro-rep to find representative sequences. Sorry for the delay again. And please show me the message if it does not work again. Thank you!

Thanks for the fix! That worked, but now striainpro-map stalls when I run it. Here's the command:
DUMPDIR=`/path/to/by/custom/taxdump/`
DBDIR=`/path/to/db/created/by/strainpro-build`
export StrainPro_DIR=`dirname ./bin/scripts/StrainPro/StrainPro-map`

./bin/scripts/StrainPro/StrainPro-map -t 8            -dump $DUMPDIR            -i $DBDIR -o output.txt   -f /path/to/read/file/file.fq.gz

I get the following messages within a few seconds, and then strainpro-map just stalls:
Load taxonomy information.
Load BWT index: /path/to/StrainPro/ref_idx/0 (1/2)
	2000000 reads have been processed (1.20% mapped) in 6 seconds
Load BWT index: /path/to/StrainPro/ref_idx/1 (2/2)
	180000 reads have been aligned in 1 seconds

It might be worth it to use continuous integration to help identify bugs in the StrainPro code. This is very simple with Travis-CI or Circle-CI. For travis-CI, you'd just need to create a .travis.yml similar to https://github.com/nick-youngblut/gtdb_to_taxdump/blob/master/.travis.yml

I'll say the bug should be the identification of LCA for two taxIDs. If they don't have any common ancestor, the identification of LCA will stall. Could you tell me where I can download your taxonomy files and your test data? Thank you!

The taxonomy can be found at http://ftp.tue.mpg.de/ebio/projects/struo/GTDB_release89/taxdump/
Can you modify the code to either directly error-out if the LCA cannot be found or at least timeout after a certain amount of time searching for the LCA?

I just updated StrainPro. Please try again and I'll test your taxonomy files.

I just updated StrainPro. Please try again and I'll test with your taxonomy files.

Do you mean that you added an error-out or time-out?

@nick-youngblut  It was a bug in StrainPro-Map. It did not exit the LCA function call when it found a common ancestor. Sorry for the mistake.

Your else break; change didn't seem to fix the issue. The strainpro-map jobs seem to still be stalling. Note: I did recreate the database via strainpro-build before retrying strainpro-map

You're right. I already checked the two taxids in the while loop. I did not pay attention. Could you give me your read file? I was wondering why it stalls. Thank you!

reads.fq.gz
Let me know if that's not enough reads to reproduce the issue

Thank you. And I also need the reference sequences.

My refs: refs.fna.gz

Thank you! I've tried with your reference sequences and your read data. It works fine but it did not output any prediction. I think the reads are not enough to reproduce the issue.

Hi, I found the customized taxonomy dump file (nodes.dmp) is not complete. The taxid of the first genome in refs.fna is 310298 which cannot be found in nodes.dmp. There is no information for that taxid. Therefore, StrainPro-map stalls when it tries to find the LCA of 310298 and another taxid. Do you have any suggestions how do I handle such cases?

Thanks for finding the bug! I used the wrong taxdump file. Those taxIDs are sub-species that I added to a different taxdump.
It would probably be best if strainpro-map just threw in error if the taxdump file is not complete.

Thank you! I've updated StrainPro to v0.9.7. It will check the completeness of the taxonomy dump files and check if every taxid in the reference database could be found in the dump files.

Thanks! That worked. A couple of notes about the output table format:

#TaxID                  #Read_count             #Depth                  #Relative_abundance     #Confidence_score

# is considered a comment character by many parsers, so this could cause some problems for users to read in the table (with column names)
@TaxRank:subspecies
@TaxRank:species
821                     82                      8                       22.16                   0.082000
853                     288                     23                      77.84                   0.235487
@TaxRank:genus
[...]

Instead of the @TaxRank: lines, why not just have a column named "TaxRank"? This would make the table a standard tab-delimited table that would be much easier to parse. Users can then easily parse the table by tax rank via tidyverse (dplyr) or other standard means.

Dear Dr. Youngblut,
That was a very good suggestion. I'll modify the output format to meet the
standard tab-delimited table.
Best regards,
Hsin-Nan
On Mon, Feb 17, 2020 at 1:34 AM Nick Youngblut<EMAIL_ADDRESS>wrote:

Thanks! That worked. A couple of notes about the output table format:
#TaxID #Read_count #Depth #Relative_abundance #Confidence_score
is considered a comment character by many parsers, so this could cause
some problems for users to read in the table (with column names)
@TaxRank:subspecies
@TaxRank:species
821                     82                      8                       22.16                   0.082000
853                     288                     23                      77.84                   0.235487
@TaxRank:genus
[...]
Instead of the @TaxRank: lines, why not just have a column named
"TaxRank"? This would make the table a standard tab-delimited table that
would be much easier to parse. Users can then easily parse the table by tax
rank via tidyverse (dplyr) or other standard means.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/hsinnan75/StrainPro/issues/5?email_source=notifications&email_token=ACOKJKON3VAVPQ2WCOCCSKDRDF2KXA5CNFSM4KN3BLIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL4ND2Y#issuecomment-586732011,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACOKJKJ4OXKDEFJNSDOTGRTRDF2KXANCNFSM4KN3BLIA
.

I've modified the output format. Please update StrainPro to v0.9.8. Thank you!

Thanks for modifying the format! While the new output will parse as a tsv, it is technically tab+space delimited, since you are using spaces to align columns. The user will have to strip off all spaces from the ends of the string-based values in each column (and then convert from character to numeric as necessary). This will cause confusion from a lot of users

Thanks for the suggestion. I've update StrainPro (still version 0.9.8). The output format is tsv.

Thanks!
One general question about the output: how is one supposed to interpret the confidence scores? I don't see any mention of confidence scores in the bioRxiv paper, and the README only states:

Confidence_score is the confidence score of that prediction.

What is a "good" confidence score? What confidence score cutoff was used for all of your evaluations in the bioRxiv paper?


Fix package toolchain list

When downloading the toolchain used to build mariner, a few packages are missing in the upstream repo:
https://packages.microsoft.com/cbl-mariner/2.0/prod/base/x86_64
Updating the version information so toolchain build can succeed fixes #32

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

[X] The toolchain has been rebuilt successfully (or no changes were made to it)
[ ] The toolchain/worker package manifests are up-to-date
[ ] Any updated packages successfully build (or no packages were changed)
[ ] Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
[ ] Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
[ ] All package sources are available
[ ] cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
[ ] LICENSE-MAP files are up-to-date (./SPECS/LICENSES-AND-NOTICES/data/licenses.json, ./SPECS/LICENSES-AND-NOTICES/LICENSES-MAP.md, ./SPECS/LICENSES-AND-NOTICES/LICENSE-EXCEPTIONS.PHOTON)
[ ] All source files have up-to-date hashes in the *.signatures.json files
[ ] sudo make go-tidy-all and sudo make go-test-coverage pass
[ ] Documentation has been updated to match any changes to the build system
[ ] Ready to merge

Summary

What does the PR accomplish, why was it needed?
Change Log

Change
Change
Change

Does this affect the toolchain?

YES/NO
Associated issues

#xxxx

Links to CVEs

https://nvd.nist.gov/vuln/detail/CVE-YYYY-XXXX

Test Methodology

Pipeline build id: xxxx

Its expected the main branch may be behind the published packages in PMC. There are a few ways I recommend working around:

Build from sources (REBUILD_TOOLCHAIN=y). You can save the produced archive aside and reference it using TOOLCHAIN_ARCHIVE=my_toolchain.tar.gz
Dev off of the rolling 2.0-stable branch (won't have the bleeding edge stuff)
git checkout 2.0-stable -- toolkit/resources/manifests/package This will just roll the manifests back

We wrote up some bits on our branches here: https://github.com/microsoft/CBL-Mariner/blob/2.0/toolkit/docs/building/building.md#clone-and-sync-to-stable-commit
Not sure if this is something you have run into, took me by surprise. Git behaves oddly (at least feels odd to me) with tags, you need to explicitly ask it to pull updates to tags via git fetch upstream --tags. It won't update the rolling tags once you have one defined locally on its own.

Ahh, apologies, I was not aware of the 2.0-stable branch, let me close this out


Stuck at "Trying to write tags" on MP4

I have convert-mp4 set to false so when i point manual.py to a directory with a mix of mkv and mp4 it gets stuck at "Trying to write tags" on an MP4 file although its supposed to skip it.

Its not converting its just writing tags, that's usual behavior
Why the file is getting stuck on trying to write tags is another issue, if you don't want tagging you can turn that feature off

I'm seeing this too.  I want it to tag existing MP4s, so this isn't about turning off that feature for me.
Where it gets stuck seems to be at an I/O system call.  Ctrl-C doesn't work to kill the process, I have to do End process through the Task Manager (Windows) and it takes a while after doing that for the process to exit.  I ran python -m trace --trace manual.py ... and the last output seems to be a file operation.
I'm reasonably certain I've seen it successfully tag other existing MP4s so it seems to be specific to certain files.  I've also been able to reproduce the hang with multiple existing MP4s in the same folder downloaded from the same source, so it's probably specific to a certain way of doing things.  The files that tagging hangs on all play just fine.
That's all the info I have, hope it helps!


Enhance empty state string for count

If a count doesn't have any items in it, the empty state should say "No items added to count"
Current:

For a hard count the progress indicator doesn't make much sense since it will always be 100% due to no expected and actual comparison.

Also for hard counts the 0/0 counted should just say the number of SKUs counted. Since there is no expected value, we can't show a fraction.


Fix malformed bashling theme file causing ST3 error

I believe this fix will address the following error in ST3.
To repro before my fix:

open command palette
type install theme
type bas
See the following error

CC: @lingtalfi the author of the bashling theme.

thank you!

Weird our tests didn't catch this.

Maybe we need tests for the tests? :sweat_smile:

@blopker @shakeelmohamed you guys are right!


WIP: Get TotalPixelMatrixOriginSequence from PlanePositionSlideSequence if not present

Original issue: https://github.com/GoogleCloudPlatform/wsi-to-dicom-converter/issues/38

@hackermd this is the current solution to get the TotalPixelMatrixOriginSequence. Even though we have the TotalPixelMatrixOriginSequence, I'm getting negative coordinates when adding annotations:
scoord3d.js?ae01:215 Uncaught Error: coordinates of Polygon must be positive numbers
    at new Polygon (scoord3d.js?ae01:215)
    at _geometry2Scoord3d (viewer.js?182d:138)
    at _getROIFromFeature (viewer.js?182d:340)
    at Draw.eval (viewer.js?182d:969)
    at Draw.Target.dispatchEvent (Target.js?af7a:114)
    at Draw.finishDrawing (Draw.js?8817:880)
    at Draw.handleUpEvent (Draw.js?8817:574)
    at Draw.PointerInteraction.handleEvent (Pointer.js?154c:144)
    at Draw.handleEvent (Draw.js?8817:524)
    at Map.PluggableMap.handleMapBrowserEvent (PluggableMap.js?7229:860)

Is that what you meant in your comment here?
Maybe there is some part of the viewer that needs to be updated?
I'm on my way to get permission to share the test data with you so you can have a better look at it.

Thanks @igoroctaviano for looking into the issue. Test data would certainly be help to debug the issue.

Thanks @igoroctaviano for looking into the issue. Test data would certainly be help to debug the issue.

In general, I am reluctant to hack around issues due to missing type I attributes. If a required attribute is missing in a data set, it is an indication that the data set may have additional standard compliance issues.

In general, I am reluctant to hack around issues due to missing type I attributes. If a required attribute is missing in a data set, it is an indication that the data set may have additional standard compliance issues.

If type 1 attributes are missing that are required for image display, the viewer should refuse to display the images. I would propose to try to fix the metadata before passing them to the viewer constructor.

If type 1 attributes are missing that are required for image display, the viewer should refuse to display the images. I would propose to try to fix the metadata before passing them to the viewer constructor.


include bower.json

when it gets merged, i'll add it to the bower registry

👍

Thanks for this :smile:

Great! It's now available via bower install currency-symbol-map. The registry will auto-updated whenever a semver (ie: v3.0.1) tag is pushed to this repo.


Added openarena 0.8.8

Haven't found the must be in repo.

@wyvie @jagajaga This broke master because stdenv.glibc doesn't exist (?!). See e.g. https://travis-ci.org/NixOS/nixpkgs/builds/45582143 (at the bottom)

@wyvie @jagajaga This broke master because stdenv.glibc doesn't exist (?!). See e.g. https://travis-ci.org/NixOS/nixpkgs/builds/45582143 (at the bottom)

@wmertens hm, strange. Travis was fine when PR was opened.

@wmertens hm, strange. Travis was fine when PR was opened.

@jagajaga are you sure? https://travis-ci.org/NixOS/nixpkgs/builds/45113418 Says it didn't...
Either way it's super strange that stdenv.glibc doesn't exist.

@jagajaga are you sure? https://travis-ci.org/NixOS/nixpkgs/builds/45113418 Says it didn't...
Either way it's super strange that stdenv.glibc doesn't exist.

@wmertens I'm tottaly sure (I'm not accepting PR if travis fails).
Yeah, that's strange :fearful:

@wmertens I'm tottaly sure (I'm not accepting PR if travis fails).
Yeah, that's strange :fearful:

@jagajaga oh sorry, the PR build was https://travis-ci.org/NixOS/nixpkgs/builds/45051966 and passed. So something between those master commits broke it and openarena was the unlucky first to trigger. Checking further.

@jagajaga oh sorry, the PR build was https://travis-ci.org/NixOS/nixpkgs/builds/45051966 and passed. So something between those master commits broke it and openarena was the unlucky first to trigger. Checking further.

@wmertens i'm definitely sure travis build was successful on my pr. i was also quite happy about that fact.


Fix cache transformation bug

If a sha was requested before the artifact the resulting transformed jar would end up corrupted.

could this change effect make e2etest and that is why that failed, or was that failure a flake or related to something else @stuartwdouglas ?

It would not affect e2etest, the cache is not used there.


File not found. Skipping.

I feel like I should be apologizing already as I am sure this is not a bug, but a pebkac.
This is on antiX Linux 21 Core (Debian 11, non-systemd base), nvidia driver: , dxvk latest, ...
Software information
World of Warcraft, trying to install DXVK into the wine prefix.
System information

GPU: GeForce GTX 1050 Ti
Driver: 515:76 [vulkan]
Wine version: winehq-staging 7.18
DXVK version: 1.10.3

Here's what I'm doing:
From unpacked release folder:
mailiner@moonbasebeta:~/Downloads/dxvk-1.10.3
$ export WINEPREFIX="/home/mainliner/.wow/"
mailiner@moonbasebeta:~/Downloads/dxvk-1.10.3
$ ./setup_dxvk.sh install
wine: configuration in L"/home/mainliner/.wow" has been updated.
/home/mainliner/Downloads/dxvk-1.10.3/x64/dxgi.dll: File not found. Skipping.
/home/mainliner/Downloads/dxvk-1.10.3/x32/dxgi.dll: File not found. Skipping.
/home/mainliner/Downloads/dxvk-1.10.3/x64/d3d9.dll: File not found. Skipping.
/home/mainliner/Downloads/dxvk-1.10.3/x32/d3d9.dll: File not found. Skipping.
/home/mainliner/Downloads/dxvk-1.10.3/x64/d3d10core.dll: File not found. Skipping.
/home/mainliner/Downloads/dxvk-1.10.3/x32/d3d10core.dll: File not found. Skipping.
/home/mainliner/Downloads/dxvk-1.10.3/x64/d3d11.dll: File not found. Skipping.
/home/mainliner/Downloads/dxvk-1.10.3/x32/d3d11.dll: File not found. Skipping.

Be gentle.

How does the content of your dxvk-1.10.3 folder look?
It includes a x32 & x64 folder with the DLLs inside both right?

Could be some ownership/permission issues on the dxvk folder or subfolders perhaps.

Could be some ownership/permission issues on the dxvk folder or subfolders perhaps.

I think the problem is the ownership of the .wine folder.
the user is logged in as "mailiner" but the home folder that contains .wine is "mainliner". Two different users perhaps?

@themainliner Did you check the permissions of the .wine in your home directory?

mine doesnt include these files

How does the content of your dxvk-1.10.3 folder look? It includes a x32 & x64 folder with the DLLs inside both right?

mine doesnt include these files

Where did you get it from

@Blisto91 Oh my bad. I git clonned the file from the repo lol. Silly mistake


Save original (pre-randomized) prompt

My workflow is usually, I have a dynamic prompt with a bunch of randomized variables to test out many different combinations of settings, then I pick one non-randomized prompt from the output and generate some variations from that
The problem is the original randomized prompt is never saved in this case, I have to remember to write it down separately or I might lose it when I want to go back to generating random things again
Instead of that I'm wondering if the original randomized prompt (with {}, etc.) could be saved alongside each generation in case I want to look at it again. Just beside the txt2img outputs per batch is fine

There are two settings available for this in the settings tab.

Save template to metadata saves the raw prompt template to the PNG metadata
Write prompts to file will write a file with the same same but .txt as an extension with the raw template as well as every prompt used in the batch.

Does that work for you?

Oh I wasn't aware of those, thanks


Towards registering models with MLJ

Further to https://github.com/JuliaAI/MLJModels.jl/issues/454, I have been asked to provide feedback on the implementation of the MLJ model interface package for models provided by RAPIDS.jl, with a view to registering the models in MLJ's model registry.
First, it's clear that a lot of effort has been put into understanding and implementing the interface, which is really awesome. I'm pretty optimistic we can close the necessary gaps to get this across the finish line.
I have not had an opportunity for a thorough review but wanted to get the ball rolling with a few comments. Only the first is more serious.

This scitype declaration for a classifier is not appropriate for an MLJ model, as users provide targets for classification in the form of categorical vectors (element scitype Finite, which includes OrderedFactor  - for ordered categoricals - and Multiclass - for unordered ones). So, for classification, the target_scitype you typically have is AbstractVector{<:Finite} unless it's binary only, which would be AbstractVector{<:Finite{2}}.  Furthermore, one needs to take into account all the classes in the pool of the target, not just those manifest as entries seen in the training vector. This is so that the  target you predict includes all classes. (See this discussion). This generally means re-encoding the data using the categorical reference integers for passing to the core model, but also somehow recording the full pool and a method to invert the conversion in the prediction phase. This is discussed in the manual but there are also lots of existing implementations to adapt, for example this DecisionTreeClassifier for probablistic predictors, and this SVM model for deterministic (point) predictions. 2.

If your clustering models have a predict, then they should predict categorical vectors. I didn't check this.

The same scitype declaration referenced in 1. also allows a Table type and I wonder if that was intenentional. Tabular targets are only used in multitarget models (one column per target). Is this actually supported in the current case?

A lot of model_metadata entries in this pkg indicate that you can use tables and matrices for input but the examples in the doc-strings only use matrices. It's fine if your model indeed can handle both (is that the case?) but generally MLJ users are used to inputs being tabular (wrapping matrices as tables if necessary) so probably the doc-strings should include tabular examples. See the DecisioinTreeClassifier docstring for examples.

This is probably an issue you want to address last, but MLJ now has a hard requirement for model docstrings, which must comply to this standard

At this early stage, you may just want to comment on these. If you do want to work on new commits to address them, can you please put them in unmerged PR, so I can more easily comment on any changes.
Happy to take a call if you think that would be efficient at this early stage of the review.

Thank you for taking the time to review the repo. I will make all the changes to bring it to the MLJ model standard. I thought that the scitypes were incorrect, so thank you for the information on that too.

You might find this new tool useful. You can use it to test your interfaces. Eventually you might want to include it in CI but for now it remains experimental.

You should use the dev branch.

Thank you for sharing. This looks like a great addition.

@ablaom Thank you again for reviewing the library. Here is a PR with some of the improvements that you suggested: #25

This scitype declaration for a classifier is not appropriate for an MLJ model, as users provide targets for classification in the form of categorical vectors (element scitype Finite, which includes OrderedFactor  - for ordered categoricals - and Multiclass - for unordered ones). So, for classification, the target_scitype you typically have is AbstractVector{<:Finite} unless it's binary only, which would be AbstractVector{<:Finite{2}}.  Furthermore, one needs to take into account all the classes in the pool of the target, not just those manifest as entries seen in the training vector. This is so that the  target you predict includes all classes. (See this discussion). This generally means re-encoding the data using the categorical reference integers for passing to the core model, but also somehow recording the full pool and a method to invert the conversion in the prediction phase. This is discussed in the manual but there are also lots of existing implementations to adapt, for example this DecisionTreeClassifier for probablistic predictors, and this SVM model for deterministic (point) predictions. 2.

Unless I am missing something, all the classification methods should be able to handle multi class classification.
I am a little confused on implementing the encoding of categorical variables. Because we pass the y vector to python, the model will be able to predict any class that is present in the training data.

If your clustering models have a predict, then they should predict categorical vectors. I didn't check this.

This should now be the case. I added the casting to MMI.categorical, because we get a normal array back from python.

The same scitype declaration referenced in 1. also allows a Table type and I wonder if that was intenentional. Tabular targets are only used in multitarget models (one column per target). Is this actually supported in the current case?

This was incorrect. I have fixed the target scitype.

A lot of model_metadata entries in this pkg indicate that you can use tables and matrices for input but the examples in the doc-strings only use matrices. It's fine if your model indeed can handle both (is that the case?) but generally MLJ users are used to inputs being tabular (wrapping matrices as tables if necessary) so probably the doc-strings should include tabular examples. See the DecisioinTreeClassifier docstring for examples.

The machines should work with table/matrix inputs for the features. I will add some examples with this.

This is probably an issue you want to address last, but MLJ now has a hard requirement for model docstrings, which must comply to this standard

I updated the docstrings for the classifiers (LogisticRegression,...). Let me know what you think.
A side note, if you do try to install it, there is a problem with micromamba installing rapids right now. The workaround is setting the solver as mamba by setting the following env variable: JULIA_CONDAPKG_EXE=mamba.
https://github.com/cjdoris/CondaPkg.jl/issues/38

@tylerjthomas9 Thanks indeed for this update. Currently swamped with reviews and JuliaCon prep, but I will return to this in due course.

Having trouble getting using RAPIDS to work. I'm using mamba as suggested above.
Any ideas?
WARNING: Method definition load_path(Type{var"#s446"} where var"#s446"<:RAPIDS.HDBSCAN) in module RAPIDS at /home/ubuntu/Julia/RAPIDS/src/cuml/clustering.jl:135 overwritten at /home/ubuntu/Julia/RAPIDS/src/cuml/dimensionality_reduction.jl:227.
  ** incremental compilation may be fatally broken for this module **

ERROR: InitError: Python: ImportError: /opt/julia-1.7.3/bin/../lib/julia/libstdc++.so.6: version `GLIBCXX_3.4.30' not found (required by /home/ubuntu/for_extra_software/julia_depot/environments/rapids/.CondaPkg/env/lib/python3.9/site-packages/pyarrow/../../../libarrow.so.700)
Python stacktrace:
 [1] <module>
   @ ~/for_extra_software/julia_depot/environments/rapids/.CondaPkg/env/lib/python3.9/site-packages/pyarrow/__init__.py:65
 [2] <module>
   @ ~/for_extra_software/julia_depot/environments/rapids/.CondaPkg/env/lib/python3.9/site-packages/cudf/core/dtypes.py:10
 [3] <module>
   @ ~/for_extra_software/julia_depot/environments/rapids/.CondaPkg/env/lib/python3.9/site-packages/cudf/api/types.py:18
 [4] <module>
   @ ~/for_extra_software/julia_depot/environments/rapids/.CondaPkg/env/lib/python3.9/site-packages/cudf/api/__init__.py:3
 [5] <module>
   @ ~/for_extra_software/julia_depot/environments/rapids/.CondaPkg/env/lib/python3.9/site-packages/cudf/__init__.py:12
Stacktrace:
 [1] pythrow()
   @ PythonCall ~/for_extra_software/julia_depot/packages/PythonCall/XgP8G/src/err.jl:94
 [2] errcheck
   @ ~/for_extra_software/julia_depot/packages/PythonCall/XgP8G/src/err.jl:10 [inlined]
 [3] pyimport(m::String)
   @ PythonCall ~/for_extra_software/julia_depot/packages/PythonCall/XgP8G/src/concrete/import.jl:11
 [4] __init__()
   @ RAPIDS ~/Julia/RAPIDS/src/RAPIDS.jl:37
 [5] _include_from_serialized(path::String, depmods::Vector{Any})
   @ Base ./loading.jl:768
 [6] _require_from_serialized(path::String)
   @ Base ./loading.jl:821
 [7] _require(pkg::Base.PkgId)
   @ Base ./loading.jl:1130
 [8] require(uuidkey::Base.PkgId)
   @ Base ./loading.jl:1013
 [9] require(into::Module, mod::Symbol)
   @ Base ./loading.jl:997
during initialization of module RAPIDS

I'm using the version on main branch.

I'm using the version on main branch.

I am trying to narrow down this issue. I can build it without issues on one of my computers, but I get the same error as you on a different computer. I'll let you know as soon as I can get the builds working everywhere again. It seems that something broke right after i published v0.1.0, and building the environment has become less consistent.

Yeah, I can appreciate this kind of multi-language GPU installation is very tricky. I will wait for you to sort this out before proceeding with a review.

The nightly builds of julia should contain the GCC updates required for the most recent versions of RAPIDS (https://github.com/JuliaLang/julia/pull/45582#event-7730373179). Additionally, the micromamba build bug has been fixed. I am going to start development on this library again now that these roadblocks are clearing up. I will let you know when it's ready for review.

Awesome. Will 1.8.3 (currently release-1.8) suffice? If so I might wait for that.

Awesome. Will 1.8.3 (currently release-1.8) suffice? If so I might wait for that.

I think the backport is scheduled to come with 1.8.4. We can circle back when that comes out.

Yes. I know this is frustrating, but I prefer to prioritise review to interfaces that will work with current Julia. Hang in there.

Yes. I know this is frustrating, but I prefer to prioritise review to interfaces that will work with current Julia. Hang in there.

Sounds good. I'll ping you when it's ready.

Hey @ablaom, I put a little time into getting this library to a similar level as CatBoost.jl. I think that it is in a good spot for a review when you have the capacity. There is absolutely no rush.
I want to eventually move away from Python and wrap the C++/CUDA code directly, but I won't have the capacity to tackle this transition for a while. Because of this eventual transition at some point in the future, I think that this would be a good addition to the third-party MLJ models.

Okay, I'm having trouble because of this error:
"ERROR: LoadError: InitError: IOError: write: no space left on device (ENOSPC)"
I've checked by disk and there is enough space. I understand this may be tricky to diagnose. Two facts lead me to kick the can on this one: my old GPU is to be replaced; after next week I am between contracts and I'm going to loose access. So I'll try again when the dust settles. Thanks for your patience.