Stage graphql_controller, generated classes and User type

This is just the controller, and the (mostly empty) base classes generated as scaffolding for a graphql API.
The User type and query, login input type and mutation have been added too. Note the User type has all its associations commented out.

Coverage decreased (-0.3%) to 92.649% when pulling e701421b3e0a487395aaf84e5644dd866ae1606a on nimmo-stage-graphql-files into 517b4ec1b634c16d94ce8a6a4e4ec144ae1cf350 on master.

Excellent, I'm looking forward to seeing this.  I appreciate this slow
introduction.  It will give me time to see what is auto-generated and not,
what the basic functionality is, etc.
On Sun, Jan 30, 2022 at 2:59 AM andrew nimmo @.***>
wrote:

This is just the controller, and the (mostly empty) base classes generated
as scaffolding for a graphql API.
The user type and query, and user input type and login mutation have been
added too.
You can view, comment on, or merge this pull request online at:
https://github.com/MushroomObserver/mushroom-observer/pull/840
Commit Summary

cde75af
https://github.com/MushroomObserver/mushroom-observer/pull/840/commits/cde75aff306f21f5e52a8937861191f1a29c725e
Move default schema_path
243c56f
https://github.com/MushroomObserver/mushroom-observer/pull/840/commits/243c56f6023ce59116289978fe7b2c57a9c1fc41
Create graphql_controller.rb

File Changes
(2 files
https://github.com/MushroomObserver/mushroom-observer/pull/840/files)

A app/controllers/graphql_controller.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840/files#diff-6e4127392699e1ef9c115431531f5f0274101dc9c8939de427ccf11549e8b931
(118)
M lib/tasks/graphql.rake
https://github.com/MushroomObserver/mushroom-observer/pull/840/files#diff-ffc49224b5abb9fee2d00d32c9209563f78560d77b7041be33df298a98bc3eda
(2)

Patch Links:

https://github.com/MushroomObserver/mushroom-observer/pull/840.patch
https://github.com/MushroomObserver/mushroom-observer/pull/840.diff

—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNMV6V5SOSKUJ2HN4S3UYTVXJANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you are subscribed to this thread.Message
ID: @.***>

You'll notice the types/models (currently, only User) parallel Rails model definitions and db schema.
For User, I just commented out all the associations because the types don't exist yet in this PR.
Question
Because GraphQL is all about data typing, I believe this is a good time to create_column_null wherever relevant in the User model --created_at is a candidate. What do you guys think?
In subsequent PRs, as each model is added to graphql's types, we can do the same, and we'll have @pellaea 's Pivotal story solved.

The types/models files are generated by a command line, but that only gets you the fields that come directly from table columns. IIRC i manually added the nullability params, checking against schema.rb.
I definitely manually added all the associations and field resolvers.

Right!  That's exactly what I mean, I think.  Thank you for thinking about
avoiding the unnecessary hit on the images table.  Same thing will apply to
the observation thumbnail image, too.
On Sun, Jan 30, 2022 at 10:32 PM andrew nimmo @.***>
wrote:

@.**** commented on this pull request.
In app/graphql/types/models/user_type.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r795321151
:

field :reviewed_name_descriptions, [Types::Models::NameDescriptionType],

null: true

# field :to_emails, [Types::Models::QueuedEmailType], null: true

has and belongs to many

field :user_groups, [Types::Models::UserGroupType], null: true

field :authored_names, [Types::Models::NameDescriptionType], null: true

field :edited_names, [Types::Models::NameDescriptionType], null: true

field :authored_locations, [Types::Models::LocationDescriptionType],

null: true

field :edited_locations, [Types::Models::LocationDescriptionType],

null: true

field :curated_herbaria, [Types::Models::HerbariumType], null: true

custom fields

field :img_src_thumb, String, null: true

Oh - the reason they're here is just so we don't have to query the (user
profile) image from the image table. Calling Image.url on the User.image_id
gives the urls without hitting the image table.
But i think i understand what you mean, they could still all be in the
Image model as methods!
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r795321151,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNJVCKQPM2AVQSZOV6LUYX7FXANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

That's a cool thing about that GraphiQL "API explorer" - you can run a query and observe every db hit, in a terminal window, as it's happening. n+1's really show up - that's how I noticed it, and why i started working on a way to avoid the hit.
I think Glossary terms and another model have images too, maybe herbarium. I will make a helper method for all these types when i figure out how to do it. The "featured" images have different column names for different models, though. It makes accessing the stored image id a bit trickier.

Something which might help.  Some or all of the polymorphism of image
rendering is encapsulated in Image::Url.  Perhaps there's a way to use it
as-is, or tweak it / beef it up so that you could reduce the repetition in
each graphql object type to a minimum.  Honestly, all it needs is an id!
For some purposes you also need, perhaps, size and/or transferred bit,
too.  But as a first pass / default, id alone should be enough for
everything.
On Sun, Jan 30, 2022 at 10:47 PM andrew nimmo @.***>
wrote:

That's a cool thing about that GraphiQL "API explorer" - you can run a
query and observe every db hit, in a terminal window, as it's happening.
n+1's really show up - that's how I noticed it, and why i started working
on a way to avoid the hit.
I think Glossary terms and another model have images too, maybe herbarium.
I will make a helper method for all these types when i figure out how to do
it. The "featured" images have different column names for different models,
though. It makes accessing the stored image id a bit trickier.
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1025350449,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNINDTP6LKY7PC4Y263UYYA33ANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

One prob, i believe in Image.url - it doesnt fill in the domain in some cases (idk which! Seems random.) Only returns a relative url.
That works on the live site but not in the separate frontend, so maybe we could add a check that it's returning a full Url.?

On Jan 30, 2022, at 8:03 PM, Jason Hollinger @.***> wrote:
Something which might help. Some or all of the polymorphism of image
rendering is encapsulated in Image::Url. Perhaps there's a way to use it
as-is, or tweak it / beef it up so that you could reduce the repetition in
each graphql object type to a minimum. Honestly, all it needs is an id!
For some purposes you also need, perhaps, size and/or transferred bit,
too. But as a first pass / default, id alone should be enough for
everything.
On Sun, Jan 30, 2022 at 10:47 PM andrew nimmo @.***>
wrote:

That's a cool thing about that GraphiQL "API explorer" - you can run a
query and observe every db hit, in a terminal window, as it's happening.
n+1's really show up - that's how I noticed it, and why i started working
on a way to avoid the hit.
I think Glossary terms and another model have images too, maybe herbarium.
I will make a helper method for all these types when i figure out how to do
it. The "featured" images have different column names for different models,
though. It makes accessing the stored image id a bit trickier.
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1025350449,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNINDTP6LKY7PC4Y263UYYA33ANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you authored the thread.

Right, that might be worth fixing so that it always returns a fully
qualified URL.  It might have something to do with the image
configuration.  Check config/image_config.yml.  If development local files
are served relative, remote files are fully-qualified.  Maybe you want to
change the local "read" spec to "http://localhost:3000/images"?  Is there
any reason to worry that this may not be same on mac versus linux, or
vagrant versus docker?  The relative address is by contrast very likely to
work on all of the above, no matter what port, etc.  Presumably that's why
I wrote it that way.  All of the production specs are fully-qualified.
On Sun, Jan 30, 2022 at 11:11 PM andrew nimmo @.***>
wrote:

One prob, i believe in Image.url - it doesnt fill in the domain in some
cases (idk which! Seems random.) Only returns a relative url.
That works on the live site but not in the separate frontend, so maybe we
could add a check that it's returning a full Url.?

On Jan 30, 2022, at 8:03 PM, Jason Hollinger @.***> wrote:
Something which might help. Some or all of the polymorphism of image
rendering is encapsulated in Image::Url. Perhaps there's a way to use it
as-is, or tweak it / beef it up so that you could reduce the repetition
in
each graphql object type to a minimum. Honestly, all it needs is an id!
For some purposes you also need, perhaps, size and/or transferred bit,
too. But as a first pass / default, id alone should be enough for
everything.
On Sun, Jan 30, 2022 at 10:47 PM andrew nimmo @.***>
wrote:

That's a cool thing about that GraphiQL "API explorer" - you can run
a
query and observe every db hit, in a terminal window, as it's
happening.
n+1's really show up - that's how I noticed it, and why i started
working
on a way to avoid the hit.
I think Glossary terms and another model have images too, maybe
herbarium.
I will make a helper method for all these types when i figure out how
to do
it. The "featured" images have different column names for different
models,
though. It makes accessing the stored image id a bit trickier.
—
Reply to this email directly, view it on GitHub
<
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1025350449
,
or unsubscribe
<
https://github.com/notifications/unsubscribe-auth/AAYTNNINDTP6LKY7PC4Y263UYYA33ANCNFSM5ND5ZQBQ

.
Triage notifications on the go with GitHub Mobile for iOS
<
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android
<
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub
.
You are receiving this because you were mentioned.Message ID:
@.***>

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you authored the thread.

—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1025360459,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNIZTVDBWE46KJWQWQTUYYDYDANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

Sorry - nevermind! Just dug a bit deeper and realize the observations in question simply did not have a thumb_image_id, and i forgot to check before sending a null id to Image.url

That would do it!
On Mon, Jan 31, 2022 at 3:59 AM andrew nimmo @.***>
wrote:

Sorry - nevermind! Just dug a bit deeper and realize the observations in
question simply did not have a thumb_image_id, and i forgot to check
before sending a null id to Image.url
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1025510500,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNIXHIRS64BQHMVQ4T3UYZFO7ANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

OK - This PR is the first to make use of the new Rails > 5.2 credentials added in #806, and it's the reason I proposed adding them. The new class generates the tokens.
However, they won't work unless the new master key, which I emailed around at the time but can easily resend, is added to the production environment. If this hasn't been added already, I believe that's why the test is failing, where it passes locally for me.
The master key is intentionally not part of the repo, so it needs to be uploaded to the production server and required in config/environments/production.rb to work on the live site.
config.require_master_key = true

I guess the master key also needs to be somehow added to codeclimate's environment so it can test using the class. I have no clue how to do this!

@JoeCohen It looks like our ci_rails.yml may be the place to somehow set the master key without putting it in the repo? I'm still researching, since there must be a standard way to do this.

Update I found various proposals for adding the Rails master.key to GitHub CI..
@JoeCohen @pellaea Do you guys like any of these, keeping in mind that of course the secret cannot live in the repo? This one looks right to me, but I think maybe only you two and Nathan can add a secret or environment variable.

My memory is that the key itself does not go into ci_rails.yml
https://github.com/MushroomObserver/mushroom-observer/blob/master/.github/workflows/ci_rails.yml
(which
is in the repo and under version control).
Rather there's a way to refer to the key so that it gets passed to
Coveralls.
It might already be there.
https://github.com/marketplace/actions/coveralls-github-action

name: Coveralls
uses: @.***
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
path-to-lcov: ./coverage/lcov/lcov.info

But I don't have time to dig into this until Mon or Tues.

@JoeCohen Thanks for checking it out, no need to respond til Monday.
I think there is an option in GitHub (for admins, but not for me) to add secrets to the repo that are not in the repo, and those secrets can get passed to CI, I believe by setting an environment variable in ci_rails.yml
As far as I can tell, ci_rails.yml is also not under version control (?? or at least not in the mushroom-observer i'm working with)

OK - I added some more test coverage (for User Query) and I believe this PR is good to go.
Now that master.key is on the production server, though we're not using it on the live site yet, I  add this comment to config/environments/production.rb so we don't forget it's necessary later.
# config.require_master_key = true

Regarding decreased coverage:
The scaffolded and yet unused base types for mutation, inputObject, interface, connection, etc are a lot of the coverage decrease. I can remove them and re-add with later PRs as necessary, it'll just be more confusing to re-require and include them as parameters where they go in other types.
Any suggestions for the controller? By the way, I remembered why the string user-id: was encoded in the token:  we may also want to put autologin: and admin-mode: in the token. (Does that sound right? The token is the only source of session state between the front end and Rails.)

I get the part about the unlikely but real possibility of a bad agent
trying to sneak a token for an unverified user in there, and definitely
agree that we need to test for that.  But I'm questioning whether to have
it happen automagically here where you lookup the user instance.  I say
we let this go for the moment and I'll wait until I see how it is
eventually used within the context of GraphQL before making any
suggestions.  I'm still also privately holding out for the possibility of
moving these methods out of User some day.  Fortunately, the method names
are distinctive and unique, so it will be easy to change our minds on this
later.  (We will be able to search for all uses of User.find_by_token and
User#create_graphql_token and modify as required easily.)  But again, I
need to understand the broader picture before I can even form an opinion
let alone make a coherent argument for any concrete change.  Just some
things that bother me and that I'm going to keep an eye on.
On Fri, Feb 11, 2022 at 1:22 PM andrew nimmo @.***>
wrote:

@.**** commented on this pull request.
In app/models/user.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r804901720
:

@@ -512,6 +515,29 @@ def verify
save
end

Create a Rails native token (used for authentication in GraphQL API)

def create_graphql_token
raise("User not verified") unless verified

Name proposal welcome. The situation i'm trying to guard against is
spam/bot registration - quick bot signup with fake email, and then sign in
with costly queries. There's no way i can imagine an unverified user should
get a token... When they first create an account, that shouldn't give them
a token either, i believe. Even on the current site, they sign up, but must
verify their email, and only then can they login. right?
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r804901720,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNIHHJILZWBXU5KNUE3U2VHWNANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

Right, I figured that the user of "user_id:" implied there would eventually
be other data included.  But in that case, we need to fix the parser.  I
think it was something like token.gsub("user_id:", "")  but that will leave
all the other data in there.  And writing a new token becomes complicated:
do we want to write a fresh new token with just one bit of data, or add a
bit of data to an existing token, or change the value of user_id in an
existing token?  If this hasn't already been anticipated and solved by
rails or GraphQL, we probably need to write a simple Token class that can
handle these sorts of requests.  Ah, but I see, doing it this way will
automatically be forward-compatible!  Bare user_id would not, the string
"user_id:1234" is.  Okay, again, I'm understanding why you've got the
quick-and-dirty gsub parser in there.  You already know that's temporary,
so no reason to sweat over it...  Okay, yet again, ignore me and proceed.
Hopefully I'll remember all these things.  (Ha, what a joke!)
On Fri, Feb 11, 2022 at 2:11 PM andrew nimmo @.***>
wrote:

Regarding decreased coverage:
The scaffolded and yet unused base types for mutation, inputObject,
interface, connection, etc are a lot of the coverage decrease. I can remove
them and re-add with later PRs as necessary, it'll just be more confusing
to re-require and include them as parameters where they go in other types.
Any suggestions for the controller? By the way, I remembered why the
string user-id: was encoded in the token: we may also want to put
autologin: and admin-mode: in the token. (Does that sound right? The
token is the only source of session state between the front end and Rails.)
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1036530968,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNJH4MTYZ5BGFT4BAG3U2VNNVANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Okay, i'm feeling like i should treat coverage as sacrosanct. I'm pulling out the yet-unused base classes.
I think i'd like to try to figure out admin_mode and autologin for the token now, so i can have your keen insight on it while you remember the whole arrangement. This can be our "auth" PR.

  But honestly, it would make the PR much easier to review.
There is so much unused stuff in there that I had trouble figuring out what
actually was used!
On Fri, Feb 11, 2022 at 2:44 PM andrew nimmo @.***>
wrote:

It's awesome that you know enough to be opinionated on these things!!
Okay, i'm feeling like i should treat coverage as sacrosanct. I'm pulling
out the yet-unused base classes.
I think i'd like to try to figure out admin_mode and autologin for the
token now, so i can have your keen insight on it while you remember the
whole arrangement. This can be our "auth" PR.
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1036558178,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNJTUMABTU6KOBXEPJTU2VRLLANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

I reckon the token should be built as a hash, converted to a string and encrypted, and then decrypted as a string and parsed to a hash for manipulations? Seems more convenient than string parsing.

Definitely.
On Fri, Feb 11, 2022 at 3:08 PM andrew nimmo @.***>
wrote:

I reckon the token should be built as a hash, converted to a string and
encrypted, and then decrypted as a string and parsed to a hash for
manipulations? Seems more convenient than string parsing.
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1036578776,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNO2RGSJIPFRZXFNJR3U2VUD5ANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

Ok - the methods are modified (certainly starting to look like a candidate for a Token class!) and the hash is parsed ok.
But i can't seem to get token_in_admin_mode? to work. It's in there commented out, but when i call it from graphql_controller it's giving
ActiveSupport::MessageVerifier::InvalidSignature in ::User.decrypt_token_hash.
I can't imagine what's wrong -  i modeled self.token_in_admin_mode?(http_auth_header) on self.get_from_token(http_auth_header) and it's passed the same header param to parse in graphql_controller. Does the signature expire once it's used?

Sorry, got it. Commit coming

Although coverage is now increased (first time i've been a part of that!), I should add some tests for invalid queries and variables to exercise the error handling side of the graphql_controller

This PR looks great now.  I've made a couple of recommendations.  See if you think they're worth it.

Memoizations in! But not sure if i did them right...

No, that's perfect.  It will only evaluate the right hand side once if it
resolve to something non-nil.
On Mon, Feb 14, 2022 at 1:08 AM andrew nimmo @.***>
wrote:

@.**** commented on this pull request.
In app/controllers/graphql_controller.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r805528345
:

user = User.safe_find(token["user_id"])

raise("User not found") unless user

Should we raise an error here? Silently giving no current_user:

return unless user.verified

user
end

def in_admin_mode?
return false unless token.is_a?(Hash)

token["in_admin_mode"]
end

def token
::Token.decrypt_from_header(http_auth_header).hash

Just attempted this -
It works if I add @token ||=
::Token.decrypt_from_header(http_auth_header).hash but not if i'm
replacing token with @token everywhere in the other methods. I don't
understand how it's helping if i don't use the instance var everywhere?
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r805528345,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNN33CQIY2NW2ET3IOLU3CL6PANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

Oh - but should @token ||= blah be @@token ||= blah also?
In this case it seems like the "class instance" part is appropriate. Each token is evaluated per instance. (For instance!)

Oh... One outstanding question (beyond this pr's scope, also):

Because GraphQL is all about data typing, maybe this is a good time to create_column_null where it's missing and relevant in the User model --created_at is a candidate?

Sooner or later having a mismatch on field nullability may create problems with GraphQL. I don't know what the situation is in the db currently, it just seemed worth raising the question.

Ah, no, because that's an instance method, not a class method.  The token
is going to be different for each GraphqlController instance (that is, for
each request).  But the encrypter in Token.crypt is going to be constant
for all requests, as it depends only on the global credentials.  I think
this is exactly right.
On Mon, Feb 14, 2022 at 1:23 AM andrew nimmo @.***>
wrote:

Oh - but should @token ||= blah be @@token ||= blah also?
In this case it seems like the "class instance" part is appropriate. Each
token is evaluated per instance. (For instance!)
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1038694330,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNLJ2NYS2556FZFDMXLU3CNUHANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

Personally, I don't think this is so important.  For one, it should be
impossible to alter the state of users.created_at.  And second, the null
thing is just for error-checking, so it's not critical.  The worst case
would be if the database won't accept null but graphql does, then the
database will throw an unexpected error.  But what's so bad about the db
throwing the error instead of graphql?  I mean, sure, if it's convenient,
fix these things as you come across them.  But I wouldn't waste much time
on it.
On Mon, Feb 14, 2022 at 1:30 AM andrew nimmo @.***>
wrote:

Oh... One outstanding question (beyond this pr's scope, also):
Because GraphQL is all about data typing, maybe this is a good time to
create_column_null where it's missing and relevant in the User model --
created_at is a candidate?
Sooner or later having a mismatch on field nullability may create problems
with GraphQL. I don't know what the situation is in the db currently, it
just seemed worth raising the question.
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1038699031,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNMDDOJLEJOQZMUEWX3U3COONANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

What brought it to mind was those last migrations making rss_logs.created_at, and making it non-nullable -- it was an extra step, but easy. I don't know what it affects, it just seems to make sense to tidy it up, as long as it's a simple migration... a basic thing that should be safe to assume, and could cause unexpected headaches in the future?
However there are many other tables in the db where created_at is nullable. Luckily (in the db snapshot I am working with) I just checked them, and I didn't find any actual null records in any of those tables. This implies maybe that the migration is not necessary -- but also that doing it might not disrupt anything.
Incidentally, reading through the schema just now i'm noticing lots of tables where there is an updated_at column but no created_at - this was the rss_logs situation too. That seems more involved to do, if it's even necessary. The nullability seems simple though. Am i missing a step?

Can we do it in a separate PR, at least?  Then if there is some unexpected
side effect or complication it will be better isolated.
On Mon, Feb 14, 2022 at 2:24 AM andrew nimmo @.***>
wrote:

What brought it to mind was those last migrations making
rss_logs.created_at, and making it non-nullable -- it was an extra step,
but easy. I don't know what it affects, it just seems to make sense to tidy
it up, as long as it's a simple migration... a basic thing that should be
safe to assume, and could cause unexpected headaches in the future?
However there are many other tables in the db where created_at is
nullable. Luckily (in the db snapshot I am working with) I just checked
them, and I didn't find any actual null records in any of those tables.
This implies maybe that the migration is not necessary -- but also that
doing it might not disrupt anything.
Incidentally, reading through the schema just now i'm noticing lots of
tables where there is an updated_at column but no created_at - this was
the rss_logs situation too. That seems more involved to do, if it's even
necessary. The nullability seems simple though. Am i missing a step?
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1038741970,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNIH2TGCKB3QSGQTUW3U3CU3DANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

Oh yeah. I didn't mean this one. I figure any db migration should be its own PR in any case.
sorry I should have put all these questions in a separate email thread!

On Feb 13, 2022, at 11:31 PM, Jason Hollinger @.***> wrote:
Can we do it in a separate PR, at least? Then if there is some unexpected
side effect or complication it will be better isolated.
On Mon, Feb 14, 2022 at 2:24 AM andrew nimmo @.***>
wrote:

What brought it to mind was those last migrations making
rss_logs.created_at, and making it non-nullable -- it was an extra step,
but easy. I don't know what it affects, it just seems to make sense to tidy
it up, as long as it's a simple migration... a basic thing that should be
safe to assume, and could cause unexpected headaches in the future?
However there are many other tables in the db where created_at is
nullable. Luckily (in the db snapshot I am working with) I just checked
them, and I didn't find any actual null records in any of those tables.
This implies maybe that the migration is not necessary -- but also that
doing it might not disrupt anything.
Incidentally, reading through the schema just now i'm noticing lots of
tables where there is an updated_at column but no created_at - this was
the rss_logs situation too. That seems more involved to do, if it's even
necessary. The nullability seems simple though. Am i missing a step?
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1038741970,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNIH2TGCKB3QSGQTUW3U3CU3DANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you authored the thread.

No worries!  Migrations do not always belong in a separate PR.  But in this
case, I agree, they can live in their very own private PR without anything
else.
On Mon, Feb 14, 2022 at 2:57 AM andrew nimmo @.***>
wrote:

Oh yeah. I didn't mean this one. I figure any db migration should be its
own PR in any case.
sorry I should have put all these questions in a separate email thread!

On Feb 13, 2022, at 11:31 PM, Jason Hollinger @.***> wrote:
Can we do it in a separate PR, at least? Then if there is some unexpected
side effect or complication it will be better isolated.
On Mon, Feb 14, 2022 at 2:24 AM andrew nimmo @.***>
wrote:

What brought it to mind was those last migrations making
rss_logs.created_at, and making it non-nullable -- it was an extra
step,
but easy. I don't know what it affects, it just seems to make sense to
tidy
it up, as long as it's a simple migration... a basic thing that should
be
safe to assume, and could cause unexpected headaches in the future?
However there are many other tables in the db where created_at is
nullable. Luckily (in the db snapshot I am working with) I just checked
them, and I didn't find any actual null records in any of those tables.
This implies maybe that the migration is not necessary -- but also that
doing it might not disrupt anything.
Incidentally, reading through the schema just now i'm noticing lots of
tables where there is an updated_at column but no created_at - this was
the rss_logs situation too. That seems more involved to do, if it's
even
necessary. The nullability seems simple though. Am i missing a step?
—
Reply to this email directly, view it on GitHub
<
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1038741970
,
or unsubscribe
<
https://github.com/notifications/unsubscribe-auth/AAYTNNIH2TGCKB3QSGQTUW3U3CU3DANCNFSM5ND5ZQBQ

.
Triage notifications on the go with GitHub Mobile for iOS
<
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675

or Android
<
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub
.
You are receiving this because you were mentioned.Message ID:
@.***>

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you authored the thread.

—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1038764791,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNKVNWXASFSSKG26K6DU3CYV3ANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

I think I recognize this.  You're talking about the bit of code removing
the "" at the end of email addresses?  The autocompleter
auto-completes not only the email address, but also the user's name in
.  Instead of removing that on the client in javascript
before submitting the form, I chose to strip it out on the server in the
controller.  I don't know the details of what's going on here, but maybe it
will be similar in the end.
On Mon, Feb 14, 2022 at 1:30 PM Joseph D. Cohen @.***>
wrote:

@.**** commented on this pull request.
In app/graphql/queries/user.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806128091
:

description "get user by argument"

type Types::Models::UserType, null: false
argument :id, Integer, required: false
argument :login, String, required: false
argument :email, String, required: false
argument :name, String, required: false

def resolve(args)

 return {} unless args

 if args.key?(:id)

   ::User.find(args[:id])

 elsif args.key?(:login)

   ::User.find_by(login: args[:login])

 elsif args.key?(:email)

   ::User.find_by(email: args[:email].sub(/ <.*>$/, ""))

I'm too dense to understand this. Is this how users who are not in admin
mode are prevented from finding a user by email?
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#pullrequestreview-882034224,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNLDGQR42RGLJC5EPHDU3FC3XANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

Haha, thanks, Joe!  See this is why we keep you around.  I even tried to
look this up, but there are conflicting opinions out there.  The issue does
seem to revolve around what people expect.  In this case there is unlikely
to be a subclass of Token.  If there were, what would it be?  Maybe we
would have multiple subclasses each with a different kind of encryption?
If so, then class instance variable is definitely the way to go!
Okay, I'll try to drill it into my thick skull: "never use @@"...  As if
I'll remember...
On Mon, Feb 14, 2022 at 12:29 PM Joseph D. Cohen @.***>
wrote:

@.**** commented on this pull request.
In app/classes/token.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806079529
:

def encrypt_to_header
token_json = @.***)
Token.crypt.encrypt_and_sign(token_json)
end

def self.decrypt_from_header(auth_header)
token = Token.crypt.decrypt_and_verify(auth_header)
hash = JSON.parse(token)
new(hash)
rescue ActiveSupport::MessageVerifier::InvalidSignature
nil
end

def self.crypt
@crypt ||= ActiveSupport::MessageEncryptor.new(

I think the relevant difference is that class variables can be modified by
subclasses, which in many circumstances can lead to unpleasant surprises.
Good examples under "What is the difference between class variables and
class instance variables?" at
https://www.ruby-lang.org/en/documentation/faq/8/
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806079529,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNJVMQCQJN7N4LJR5WDU3E3XTANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

I think you're right, unless you think that there will be a subclass which
will need to change it.
On Mon, Feb 14, 2022 at 4:18 PM andrew nimmo @.***>
wrote:

@.**** commented on this pull request.
In app/classes/token.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806246202
:

def encrypt_to_header
token_json = @.***)
Token.crypt.encrypt_and_sign(token_json)
end

def self.decrypt_from_header(auth_header)
token = Token.crypt.decrypt_and_verify(auth_header)
hash = JSON.parse(token)
new(hash)
rescue ActiveSupport::MessageVerifier::InvalidSignature
nil
end

def self.crypt
@crypt ||= ActiveSupport::MessageEncryptor.new(

Ok so i'm going back to @crypt here then?
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806246202,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAALDFEXWXUGHTESFAG5GNTU3FWQTANCNFSM5ND5ZQBQ
.
You are receiving this because you were mentioned.Message ID:
@.***>

Am I missing something... this is already @crypt, right?  But yes, I defer
to Joseph on this -- he's so much better at finding out what the consensus
/ current best practices in the ruby/rails community.
On Mon, Feb 14, 2022 at 4:20 PM Joseph D. Cohen @.***>
wrote:

I think you're right, unless you think that there will be a subclass which
will need to change it.
On Mon, Feb 14, 2022 at 4:18 PM andrew nimmo @.***>
wrote:

@.**** commented on this pull request.
In app/classes/token.rb
<
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806246202
:

def encrypt_to_header
token_json = @.***)
Token.crypt.encrypt_and_sign(token_json)
end

def self.decrypt_from_header(auth_header)
token = Token.crypt.decrypt_and_verify(auth_header)
hash = JSON.parse(token)
new(hash)
rescue ActiveSupport::MessageVerifier::InvalidSignature
nil
end

def self.crypt
@crypt ||= ActiveSupport::MessageEncryptor.new(

Ok so i'm going back to @crypt here then?
—
Reply to this email directly, view it on GitHub
<
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806246202
,
or unsubscribe
<
https://github.com/notifications/unsubscribe-auth/AAALDFEXWXUGHTESFAG5GNTU3FWQTANCNFSM5ND5ZQBQ
.
You are receiving this because you were mentioned.Message ID:
@.***>

—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#issuecomment-1039573377,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNL4RE4TZFKIG33SHDTU3FW27ANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

You're right about case. I wasn't paying attention.
On Mon, Feb 14, 2022 at 4:29 PM andrew nimmo @.***>
wrote:

@.**** commented on this pull request.
In app/graphql/queries/user.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806253817
:

@@ -0,0 +1,26 @@
+# frozen_string_literal: true

+module Queries

class User < Queries::BaseQuery
description "get user by argument"
type Types::Models::UserType, null: false
argument :id, Integer, required: false
argument :login, String, required: false
argument :email, String, required: false
argument :name, String, required: false

def resolve(args)

 return {} unless args

 if args.key?(:id)

I couldn't figure out a way to phrase this with case, please let me know
if something jumps out. It's intended to be a versatile query that can be
used with one arg only.
The processing advantage of case as i understand it would be that the
case predicate is only evaluated once, and then it's compared to the when
s.
But here it's looking for a different key in a small array each time, and
i couldn't think of a way to process the args in a way that the whens
wouldn't just be looking in some other derived array.
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806253817,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAALDFHCEPY72O5LWFG22GDU3FX5HANCNFSM5ND5ZQBQ
.
You are receiving this because you were mentioned.Message ID:
@.***>

Yes, I thought so, thanks for clarifying that, Nimmo!
On Mon, Feb 14, 2022 at 4:38 PM andrew nimmo @.***>
wrote:

@.**** commented on this pull request.
In test/graphql/user/user_query_test.rb
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806259287
:

   query($id: Int){

     user(id: $id) {

       name

       id

       email

       login

     }

   }

 GRAPHQL

 user = rolf

 result = MushroomObserverSchema.execute(

   query_string, variables: { id: user.id }

 )

 user_result = result["data"]["user"]

Yes - hahaha! This is not really over your head is it Joe?!
As of this PR nothing is exposed to the internet by the graphql api, but
you're right on. Field authorization (more granular than, but in addition
to, query and mutation auth) is one of the main things we need to build in
on the Rails side.
Next PR will introduce protected fields, and we'll need to implement them
on User. Or if you guys prefer i can add them to this PR once I figure it
out. It's pretty simple.
—
Reply to this email directly, view it on GitHub
https://github.com/MushroomObserver/mushroom-observer/pull/840#discussion_r806259287,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAYTNNJEVWE7GBHC3DAWQN3U3FY3RANCNFSM5ND5ZQBQ
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID:
@.***>

I just demoted @@crypt to @crypt in the latest commit. Should be good to go, if you guys approve.
(I've been playing with field authorization, but it's not working yet.)

Done, thanks, Nimmo!

Done, thanks, Nimmo!


Add application count component test

What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?:

NONE

/approve


Replaced Arc RB mention with Arc VM management

Also updated the hyperlink.

@JasonGerend

Can you review this PR?
IMPORTANT: When this content is ready to merge, you must add #sign-off in a comment or the approval may get overlooked.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team


Add US State to issues

IMPORTANT: PRs without the template will be CLOSED
Description of your pull request and other information

Because the USA is so big and the states vary, we need to put this reminder to also include your state.
Fixes #
Template

Before submitting a pull request make sure you have:

[x] At least skimmed through contributing guidelines including yt-dlp coding conventions
[x] Searched the bugtracker for similar pull requests
[x] Checked the code with flake8 and ran relevant tests

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check one of the following options:

[x] I am the original author of this code and I am willing to release it under Unlicense
[ ] I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

[ ] Fix or improvement to an extractor (Make sure to add/update tests)
[ ] New extractor (Piracy websites will not be accepted)
[x] Core bug fix/improvement
[ ] New feature (It is strongly recommended to open an issue first)

What?!

What?!

To the issue template, I required you to put the US state you reside in if you are in the USA.

Why?

Why?

Just in case, the USA is the only major country with major division of states online. For example one site may be blocked in the east but availability in the west

Wait nevermind if it doesn’t work just say the state mabually


Bug: components still use the old props design

Describe the bug
In https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292707 the old way of making props:
export let prop1
export let prop2

was replaced with this:
export let data
let {prop1,prop2} = data

But this has not been changed for components imported from $lib
Reproduction
Reproduction
Logs
n/a

System Info
System:
    OS: macOS 12.4
    CPU: (8) x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
    Memory: 4.14 GB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    npm: 8.5.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 104.0.5112.101
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.66
    @sveltejs/kit: next => 1.0.0-next.443
    svelte: ^3.44.0 => 3.49.0
    vite: ^3.0.4 => 3.0.9

Severity
annoyance
Additional Information
If this is kept the same, it should be specified in the docs. Right now it's not, and it took me an hour to figure out why my website was broken

It's only +page.svelte and +layout.svelte that had their props replaced with a single data prop. For any other component, you can pass whatever props in that you want. I don't think there's anything here that should be documented differently. The existing Svelte tutorial https://svelte.dev/tutorial makes it clear how props work. And the SvelteKit routing migration script will only touch page and layout components, and will leave the other ones alone.

Why would you have 2 different kinds of props though? To me it seems like they should be the same

+page.svelte and +layout.svelte are special components to SvelteKit, and they adhere to a contract which says "all data is passed into the data prop". You other components can define the props interface however they want. Closing since this works as expected.

This should at least be clarified in the docs. It's incredibly confusing to have 2 different ways of doing it


Disabling failing test pending investigation

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
7 unchanged lines in 2 files lost coverage.
Overall coverage decreased (-0.02%) to 48.26%

Files with Coverage Reduction
New Missed Lines
%

extensions/notebook/src/book/bookTreeView.ts
2
38.46%

extensions/sql-database-projects/src/tools/autorestHelper.ts
5
50.0%

Totals

Change from base Build<PHONE_NUMBER>:
-0.02%

Covered Lines:
14747

Relevant Lines:
29016

💛  - Coveralls


Execute make target consecutively even on failure

What type of PR is this?
/kind feature
What does does this PR do / why we need it:
This PR enables the next make target execution even if the the existing one fails. For ex - If make test-integration fails It won't fail the overall execution, instead start the next make target execution.
Which issue(s) this PR fixes:
Fixes NA (Interop team has particularly urged for this feature as part of https://github.com/openshift/odo/issues/4223 )
PR acceptance criteria:

[ ] Unit test

[ ] Integration test

[ ] Documentation

[x] I have read the test guidelines

How to test changes / Special notes to the reviewer:
Prow should not fail overall execution. It should try executing the next target if existing one fails.

So we can see the PR behaves as expected https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_odo/4324/pull-ci-openshift-odo-master-v4.6-integration-e2e/1341333359082606592/artifacts/integration-e2e/integration-e2e-steps/container-logs/test.log . If any of the test fails jobs state shows failed, however it does not block the make target. The purpose of intentionally introduced failure has been achieved, so reverting the commit.

Prow failure dur to https://github.com/openshift/odo/issues/3256 .
Detailed log:  https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_odo/4324/pull-ci-openshift-odo-master-v4.6-integration-e2e/1341429756121321472#1:build-log.txt%3A1272
/retest

/retest


Add acceleration to JointState

We noticed that we need to transmit accelerations as well for use in controllers. No other message package (e.g. xpp_msgs) fits the full message set we need to transmit. Adding acceleration to the JointState is the easiest way to expose this extra information. Unfortunately this makes previous messages incompatible again.

I am not convinced that we should add this extra information as it is already contained in the whole-body message.
Note that you can retrieve the generalized joint acceleration from the linear momentum. This is indeed how I do in our whole-body controller.
Is there a reason why you think we should still include the joint acceleration?

Regarding the redundancy argument: The same could be said for not including the position/velocities of the end-effectors (which currently are and could be retrieved from forward kinematics).
The reason to include the extra field is that on some robots we can get the measured acceleration from hardware interfaces (which may be finite differenced and smoothed at a higher rate) and that we are using it in controllers directly. Having it as part of JointState allows us to switch these controllers to use this package as well instead of introducing new messages or message packets in other places.
Having to use linear momentum means on one side to take acceleration and compute it and then on the other side to do the opposite computation to retrieve it back again.

Hi Carlos,
What I'm trying to do is send the computed joint accelerations (computed through croc) into our WBC. I'm not sure it makes sense to re-compute the joint acceleration from linear momentum given we're already getting these are outputs from Crocoddyl.

Guys -- I have changed my mind. It would be not easy to compute the joint acceleration as we need the time derivatives of the CMM. Instead, it is still trivial to compute the CoM acceleration.
In short, please add the migration rules.
(@wxmerkt -- we might still want to rethink our contact-state msg as it is not nice to have information redundancy)

Let's increase the version number too

Apologies for the delay - I've just added the migration rule and updated the version number.

@cmastalli I've added the migration rule - is there anything else needed?

Note: Travis as configured no longer works, it will need replacing

What do we need to do for that?

I may add a new CI in the future. I dont think we necessarily need one for a message package :-D

Added a new CI - #9

Thanks! Let's merge this PR!


Add run length encoding to contract binary format for x86

With x86 programs, there is especially a lot of runs of 0. We should optimize the format used on the blockchain by using a very simple run length encoding system. With this, only the byte 0 would be affected by this, as other values are too random to benefit.
Basic psuedo code:
Loop through bytedata
if data is 0 and the next data is also 0, then error.
If data is 0, then write 0 next data times, and then advance data to next value.
Otherwise, write data.
Advance data to next value
end loop
Because there is already size fields in the binary stream, no dynamic allocations are needed and this should require only one cheap scan through the data.

done


Make kMaxStackDepth configurable

I'm profiling a program which has a stack depth of more than 64 frames. When a stack of over 64 frames is seen, the profiler will omit the highest frames in the call stack in order to limit the overall call stack size to 64 frames. This behavior interacts poorly with pprof's flamegraph visualization (see https://github.com/google/pprof/issues/537), and I would like a way to generate a profile with full stacks.
In gperftools the max call stack size is a constant and is configured by kMaxStackDepth here.
It would be great if kMaxStackDepth were configurable at runtime, and/or if it were set by default to a value high enough that almost any non-recursive program won't hit the limit (e.g. 256).
Repro instructions
(copied from this issue)

Save this program locally as main.cc. This program has a series of nested functions, main -> func1 -> func2 ... -> func62, and func62 calls square_sum which does some work and also calls square which does more work.
Compile with g++ -g -O0 main.cc.
Run with LD_PRELOAD="/usr/lib/libprofiler.so.0" CPUPROFILE=test.cpuprof ./a.out.
Dump profile contents with pprof -raw test.cpuprof.

Expected result:
Some frame stacks have at least 65 frames.
Actual result:
All frame stacks have at most 64 frames.

Yes, having more frames would be nice. Seems hard to make it completely runtime tweakable, but I would accept patch to make it (statically) bigger.

I opened a PR: https://github.com/gperftools/gperftools/pull/1203

Applied. Thanks a lot.


Added Link Shortener Tool

I have created and added the link or URL shortener method or tool where users can easily shorten their long link with one click and make it very short for sharing with others.

Hey, my contribution has been successfully added to the Hacktoberfest event. Can you please merge it now?


[WIP] Added tests for missing bonds, angles, and torsions

This adds tests for missing bonds, angles, and torsions.
Fixes #170
@davidlmobley : Can you add tests now?

There's a bug in bond checking. Fixing now.

@jchodera - I just added tests. I added one testing function which checks to see that:

Missing bonded terms result in an exception
Missing angle terms result in an exception
Missing torsion terms result in an exception
Missing LJ (Nonbonded) parameters result in an exception

You don't seem to have added a check for item 4, so missing the tests currently fail unless this portion is commented out. Can you add a check for item 4?

Also, can you NOT check for missing i-j-k-i torsions in three-membered rings? https://github.com/open-forcefield-group/smarty/issues/170#issuecomment-256409575
It seems that we are not currently assigning these (which is good, as that, as you point out, makes more sense!) and neither are AMBER forcefields.

You don't seem to have added a check for item 4, so the tests currently fail unless this portion is commented out. Can you add a check for item 4?

Adding now!

You don't seem to have added a check for item 4, so the tests currently fail unless this portion is commented out. Can you add a check for item 4?

Added a check for Lennard-Jones terms.
What should we do about charges? I think we can just ensure that every molecule in the Topology has a corresponding OEMol provided. Does that make sense?

Also, can you NOT check for missing i-j-k-i torsions in three-membered rings?

i-j-k-i torsions are now excluded.

Travis is failing to get the openeye toolkits:
3.23s$ pip install --pre -i https://pypi.anaconda.org/openeye/label/beta/simple openeye-toolkits && python -c "import openeye; print(openeye.__version__)"
Collecting openeye-toolkits
  Downloading https://pypi.anaconda.org/openeye/label/beta/simple/openeye-toolkits/2016.10b7/OpenEye-toolkits-2016.10b7.tar.gz
Collecting OpenEye-toolkits-python2.7-ubuntu-12.04-x64==2016.10b7 (from openeye-toolkits)
  Could not find a version that satisfies the requirement OpenEye-toolkits-python2.7-ubuntu-12.04-x64==2016.10b7 (from openeye-toolkits) (from versions: 2016.10b1, 2016.10b2, 2016.10b3, 2016.10b4, 2016.10b5, 2016.10b6)
No matching distribution found for OpenEye-toolkits-python2.7-ubuntu-12.04-x64==2016.10b7 (from openeye-toolkits)

What should we do about charges? I think we can just ensure that every molecule in the Topology has a corresponding OEMol provided. Does that make sense?

Sounds good to me.
On travis -- sometimes OpenEye's beta builds go up not simultaneously, so probably the appropriate beta is not up yet. It'll probably be up in a day or two.

@jchodera - is this at the point where it should be merged if tests pass locally?

Yes! It works for me, except for an absolutely insane number of OpenEye warnings:
Warning: data type mismatch found when using generic data

But please test locally on your machine!

Yes, I've gotten those warnings too; I don't know where they come from (they arrived a while back when you added a larger number of tests, I think). I'll test.

It has to do with replacing tag data with a different type:
https://docs.eyesopen.com/toolkits/python/oechemtk/genericdata.html
I wonder if it's connected to the fractional bonds.

I wonder if it's connected to the fractional bonds.

Hmm, it could be. I do use SetData, but I just do it to store back into bond orders the same data that was originally there (without changing its type); it's not clear to me why that would trigger this warning. (This is to allow charge calculation to NOT modify the original molecule.)
So, this is likely ignorable. I wish it were easier to suppress specific OE warnings though.

Tests work for me though (I ran all of them!). I'll merge.


Adding links from example README to demo.juttle.io

@mstemm - added links for better discovery of demo.juttle.io examples.
I also intend to add the same links from each adapter's README.

Instead of having nearly identical text for each of the subdirectory READMEs, maybe you could just add demo.juttle.io links to the table of the top level README?

How about I do that in addition, not instead. Who knows how people will navigate the examples tree, I can imagine them heading directly to the README of the thing they are interested in.

I guess so. I never thought of the subdirectory READMEs as standalone documents that could be read on their own, though.

If you keep the subdirectory text, can you move it into the Juttle Programs section? Something like "you can also view these programs at demo.juttle.io".

You know, if viewing the individual READMEs as potentially standalone, I can see how they are missing something. Let's keep the line up top in Setup but rephrase as "You can view this example on the demo system (link) or run it on your own using docker (see the parent README (link)).".

@mstemm ok now?

I'd still vote for moving the subdirectory demo text closer to the description of programs, but I'll defer to you if you want to keep it the way it is.
Also the demo column of the table seems kinda ugly, but I can't think of anything better.
lgtm.

Can't think of anything better either, so merging.


Swin-Transformer,VGG等模型在微调时设置frozen_stages不为-1时，使用多个gpu训练会产生错误，报告显示forward输出部分没有参与到loss计算

复现流程
bash tools/dist_train.sh configs/tutorial/vgg19-finetune-cifar.py 4       *****************************************
Setting OMP_NUM_THREADS environment variable for each process to be 1 in default, to avoid your system being overloaded, please further tune the variable for optimal performance in your application as needed.

/export/lzh/open_mmlab/mmclassification/mmcls/utils/setup_env.py:42: UserWarning: Setting MKL_NUM_THREADS environment variable for each process to be 1 in default, to avoid your system being overloaded, please further tune the variable for optimal performance in your application as needed.
warnings.warn(
/export/lzh/open_mmlab/mmclassification/mmcls/utils/setup_env.py:42: UserWarning: Setting MKL_NUM_THREADS environment variable for each process to be 1 in default, to avoid your system being overloaded, please further tune the variable for optimal performance in your application as needed.
warnings.warn(
/export/lzh/open_mmlab/mmclassification/mmcls/utils/setup_env.py:42: UserWarning: Setting MKL_NUM_THREADS environment variable for each process to be 1 in default, to avoid your system being overloaded, please further tune the variable for optimal performance in your application as needed.
warnings.warn(
/export/lzh/open_mmlab/mmclassification/mmcls/utils/setup_env.py:42: UserWarning: Setting MKL_NUM_THREADS environment variable for each process to be 1 in default, to avoid your system being overloaded, please further tune the variable for optimal performance in your application as needed.
warnings.warn(
2022-02-20 22:53:11,998 - mmcls - INFO - Environment info:
sys.platform: linux
Python: 3.8.12 (default, Oct 12 2021, 13:49:34) [GCC 7.5.0]
CUDA available: True
GPU 0,1,2,3: TITAN RTX
CUDA_HOME: /usr/local/cuda
NVCC: Cuda compilation tools, release 9.0, V9.0.176
GCC: gcc (GCC) 6.1.0
PyTorch: 1.8.0
PyTorch compiling details: PyTorch built with:

GCC 7.3
C++ Version: 201402
Intel(R) oneAPI Math Kernel Library Version 2021.4-Product Build 20210904 for Intel(R) 64 architecture applications
Intel(R) MKL-DNN v1.7.0 (Git Hash 7aed236906b1f7a05c0917e5257a1af05e9ff683)
OpenMP 201511 (a.k.a. OpenMP 4.5)
NNPACK is enabled
CPU capability usage: AVX2
CUDA Runtime 10.1
NVCC architecture flags: -gencode;arch=compute_37,code=sm_37;-gencode;arch=compute_50,code=sm_50;-gencode;arch=compute_60,code=sm_60;-gencode;arch=compute_61,code=sm_61;-gencode;arch=compute_70,code=sm_70;-gencode;arch=compute_75,code=sm_75;-gencode;arch=compute_37,code=compute_37
CuDNN 7.6.3
Magma 2.5.2
Build settings: BLAS_INFO=mkl, BUILD_TYPE=Release, CUDA_VERSION=10.1, CUDNN_VERSION=7.6.3, CXX_COMPILER=/opt/rh/devtoolset-7/root/usr/bin/c++, CXX_FLAGS= -Wno-deprecated -fvisibility-inlines-hidden -DUSE_PTHREADPOOL -fopenmp -DNDEBUG -DUSE_KINETO -DUSE_FBGEMM -DUSE_QNNPACK -DUSE_PYTORCH_QNNPACK -DUSE_XNNPACK -O2 -fPIC -Wno-narrowing -Wall -Wextra -Werror=return-type -Wno-missing-field-initializers -Wno-type-limits -Wno-array-bounds -Wno-unknown-pragmas -Wno-sign-compare -Wno-unused-parameter -Wno-unused-variable -Wno-unused-function -Wno-unused-result -Wno-unused-local-typedefs -Wno-strict-overflow -Wno-strict-aliasing -Wno-error=deprecated-declarations -Wno-stringop-overflow -Wno-psabi -Wno-error=pedantic -Wno-error=redundant-decls -Wno-error=old-style-cast -fdiagnostics-color=always -faligned-new -Wno-unused-but-set-variable -Wno-maybe-uninitialized -fno-math-errno -fno-trapping-math -Werror=format -Wno-stringop-overflow, LAPACK_INFO=mkl, PERF_WITH_AVX=1, PERF_WITH_AVX2=1, PERF_WITH_AVX512=1, TORCH_VERSION=1.8.0, USE_CUDA=ON, USE_CUDNN=ON, USE_EXCEPTION_PTR=1, USE_GFLAGS=OFF, USE_GLOG=OFF, USE_MKL=ON, USE_MKLDNN=ON, USE_MPI=OFF, USE_NCCL=ON, USE_NNPACK=ON, USE_OPENMP=ON,

TorchVision: 0.9.0
OpenCV: 4.5.5
MMCV: 1.4.4
MMCV Compiler: GCC 7.3
MMCV CUDA Compiler: 10.1
MMClassification: 0.20.1+a7f8e96
2022-02-20 22:53:11,999 - mmcls - INFO - Distributed training: True
2022-02-20 22:53:12,129 - mmcls - INFO - Config:
dataset_type = 'CIFAR10'
img_norm_cfg = dict(
mean=[125.307, 122.961, 113.8575],
std=[51.5865, 50.847, 51.255],
to_rgb=False)
train_pipeline = [
dict(type='RandomCrop', size=32, padding=4),
dict(type='RandomFlip', flip_prob=0.5, direction='horizontal'),
dict(type='Resize', size=224),
dict(
type='Normalize',
mean=[125.307, 122.961, 113.8575],
std=[51.5865, 50.847, 51.255],
to_rgb=False),
dict(type='ImageToTensor', keys=['img']),
dict(type='ToTensor', keys=['gt_label']),
dict(type='Collect', keys=['img', 'gt_label'])
]
test_pipeline = [
dict(type='Resize', size=224),
dict(
type='Normalize',
mean=[125.307, 122.961, 113.8575],
std=[51.5865, 50.847, 51.255],
to_rgb=False),
dict(type='ImageToTensor', keys=['img']),
dict(type='Collect', keys=['img'])
]
data = dict(
samples_per_gpu=64,
workers_per_gpu=2,
train=dict(
type='CIFAR10',
data_prefix='data/cifar10',
pipeline=[
dict(type='RandomCrop', size=32, padding=4),
dict(type='RandomFlip', flip_prob=0.5, direction='horizontal'),
dict(type='Resize', size=224),
dict(
type='Normalize',
mean=[125.307, 122.961, 113.8575],
std=[51.5865, 50.847, 51.255],
to_rgb=False),
dict(type='ImageToTensor', keys=['img']),
dict(type='ToTensor', keys=['gt_label']),
dict(type='Collect', keys=['img', 'gt_label'])
]),
val=dict(
type='CIFAR10',
data_prefix='data/cifar10',
pipeline=[
dict(type='Resize', size=224),
dict(
type='Normalize',
mean=[125.307, 122.961, 113.8575],
std=[51.5865, 50.847, 51.255],
to_rgb=False),
dict(type='ImageToTensor', keys=['img']),
dict(type='Collect', keys=['img'])
],
test_mode=True),
test=dict(
type='CIFAR10',
data_prefix='data/cifar10',
pipeline=[
dict(type='Resize', size=224),
dict(
type='Normalize',
mean=[125.307, 122.961, 113.8575],
std=[51.5865, 50.847, 51.255],
to_rgb=False),
dict(type='ImageToTensor', keys=['img']),
dict(type='Collect', keys=['img'])
],
test_mode=True))
checkpoint_config = dict(interval=1)
log_config = dict(interval=100, hooks=[dict(type='TextLoggerHook')])
dist_params = dict(backend='nccl')
log_level = 'INFO'
load_from = None
resume_from = None
workflow = [('train', 1)]
model = dict(
type='ImageClassifier',
backbone=dict(
type='VGG',
depth=19,
num_classes=10,
frozen_stages=2,
out_indices=(-1, ),
init_cfg=dict(
type='Pretrained',
checkpoint=
'checkpoints/vgg19_batch256_imagenet_20210208-e6920e4a.pth',
prefix='backbone')),
neck=None,
head=dict(
type='ClsHead',
loss=dict(type='CrossEntropyLoss', loss_weight=1.0),
topk=(1, 5)))
optimizer = dict(type='SGD', lr=0.01, momentum=0.9, weight_decay=0.0001)
optimizer_config = dict(grad_clip=None)
lr_config = dict(policy='step', step=[15])
runner = dict(type='EpochBasedRunner', max_epochs=20)
work_dir = './work_dirs/vgg19-finetune-cifar'
gpu_ids = range(0, 4)
2022-02-20 22:53:15,406 - mmcls - INFO - Set random seed to<PHONE_NUMBER>, deterministic: False
2022-02-20 22:53:16,825 - mmcls - INFO - initialize VGG with init_cfg {'type': 'Pretrained', 'checkpoint': 'checkpoints/vgg19_batch256_imagenet_20210208-e6920e4a.pth', 'prefix': 'backbone'}
2022-02-20 22:53:16,825 - mmcv - INFO - load backbone in model from: checkpoints/vgg19_batch256_imagenet_20210208-e6920e4a.pth
load checkpoint from local path: checkpoints/vgg19_batch256_imagenet_20210208-e6920e4a.pth
load checkpoint from local path: checkpoints/vgg19_batch256_imagenet_20210208-e6920e4a.pth
load checkpoint from local path: checkpoints/vgg19_batch256_imagenet_20210208-e6920e4a.pth
load checkpoint from local path: checkpoints/vgg19_batch256_imagenet_20210208-e6920e4a.pth
2022-02-20 22:53:17,337 - mmcv - WARNING - The model and loaded state dict do not match exactly
size mismatch for classifier.6.weight: copying a param with shape torch.Size([1000, 4096]) from checkpoint, the shape in current model is torch.Size([10, 4096]).
size mismatch for classifier.6.bias: copying a param with shape torch.Size([1000]) from checkpoint, the shape in current model is torch.Size([10]).
2022-02-20 22:53:19,785 - mmcls - INFO - Start running, host: lzh@dellt630b, work_dir: /export/lzh/open_mmlab/mmclassification/work_dirs/vgg19-finetune-cifar
2022-02-20 22:53:19,785 - mmcls - INFO - Hooks will be executed in the following order:
before_run:
(VERY_HIGH   ) StepLrUpdaterHook
(NORMAL      ) CheckpointHook
(LOW         ) DistEvalHook
(VERY_LOW    ) TextLoggerHook
before_train_epoch:
(VERY_HIGH   ) StepLrUpdaterHook
(NORMAL      ) DistSamplerSeedHook
(LOW         ) IterTimerHook
(LOW         ) DistEvalHook
(VERY_LOW    ) TextLoggerHook
before_train_iter:
(VERY_HIGH   ) StepLrUpdaterHook
(LOW         ) IterTimerHook
(LOW         ) DistEvalHook
after_train_iter:
(ABOVE_NORMAL) DistOptimizerHook
(NORMAL      ) CheckpointHook
(LOW         ) IterTimerHook
(LOW         ) DistEvalHook
(VERY_LOW    ) TextLoggerHook
after_train_epoch:
(NORMAL      ) CheckpointHook
(LOW         ) DistEvalHook
(VERY_LOW    ) TextLoggerHook
before_val_epoch:
(NORMAL      ) DistSamplerSeedHook
(LOW         ) IterTimerHook
(VERY_LOW    ) TextLoggerHook
before_val_iter:
(LOW         ) IterTimerHook
after_val_iter:
(LOW         ) IterTimerHook
after_val_epoch:
(VERY_LOW    ) TextLoggerHook
after_run:
(VERY_LOW    ) TextLoggerHook
2022-02-20 22:53:19,786 - mmcls - INFO - workflow: [('train', 1)], max: 20 epochs
2022-02-20 22:53:19,786 - mmcls - INFO - Checkpoints will be saved to /export/lzh/open_mmlab/mmclassification/work_dirs/vgg19-finetune-cifar by HardDiskBackend.
Traceback (most recent call last):
File "tools/train.py", line 194, in
main()
File "tools/train.py", line 182, in main
train_model(
File "/export/lzh/open_mmlab/mmclassification/mmcls/apis/train.py", line 208, in train_model
runner.run(data_loaders, cfg.workflow)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 127, in run
epoch_runner(data_loaders[i], **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 50, in train
self.run_iter(data_batch, train_mode=True, **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 29, in run_iter
outputs = self.model.train_step(data_batch, self.optimizer,
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/parallel/distributed.py", line 42, in train_step
and self.reducer._rebuild_buckets()):
RuntimeError: Expected to have finished reduction in the prior iteration before starting a new one. This error indicates that your module has parameters that were not used in producing loss. You can enable unused parameter detection by (1) passing the keyword argument find_unused_parameters=True to torch.nn.parallel.DistributedDataParallel; (2) making sure all forward function outputs participate in calculating loss. If you already have done the above two steps, then the distributed data parallel module wasn't able to locate the output tensors in the return value of your module's forward function. Please include the loss function and the structure of the return value of forward of your module when reporting this issue (e.g. list, dict, iterable).
Traceback (most recent call last):
File "tools/train.py", line 194, in
main()
File "tools/train.py", line 182, in main
train_model(
File "/export/lzh/open_mmlab/mmclassification/mmcls/apis/train.py", line 208, in train_model
runner.run(data_loaders, cfg.workflow)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 127, in run
epoch_runner(data_loaders[i], **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 50, in train
self.run_iter(data_batch, train_mode=True, **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 29, in run_iter
outputs = self.model.train_step(data_batch, self.optimizer,
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/parallel/distributed.py", line 42, in train_step
and self.reducer._rebuild_buckets()):
RuntimeError: Expected to have finished reduction in the prior iteration before starting a new one. This error indicates that your module has parameters that were not used in producing loss. You can enable unused parameter detection by (1) passing the keyword argument find_unused_parameters=True to torch.nn.parallel.DistributedDataParallel; (2) making sure all forward function outputs participate in calculating loss. If you already have done the above two steps, then the distributed data parallel module wasn't able to locate the output tensors in the return value of your module's forward function. Please include the loss function and the structure of the return value of forward of your module when reporting this issue (e.g. list, dict, iterable).
Traceback (most recent call last):
File "tools/train.py", line 194, in
Traceback (most recent call last):
File "tools/train.py", line 194, in
main()
File "tools/train.py", line 182, in main
main()
File "tools/train.py", line 182, in main
train_model(
File "/export/lzh/open_mmlab/mmclassification/mmcls/apis/train.py", line 208, in train_model
train_model(
File "/export/lzh/open_mmlab/mmclassification/mmcls/apis/train.py", line 208, in train_model
runner.run(data_loaders, cfg.workflow)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 127, in run
runner.run(data_loaders, cfg.workflow)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 127, in run
epoch_runner(data_loaders[i], **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 50, in train
self.run_iter(data_batch, train_mode=True, **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 29, in run_iter
epoch_runner(data_loaders[i], **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 50, in train
outputs = self.model.train_step(data_batch, self.optimizer,
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/parallel/distributed.py", line 42, in train_step
self.run_iter(data_batch, train_mode=True, **kwargs)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/runner/epoch_based_runner.py", line 29, in run_iter
and self.reducer._rebuild_buckets()):
outputs = self.model.train_step(data_batch, self.optimizer,RuntimeError
: Expected to have finished reduction in the prior iteration before starting a new one. This error indicates that your module has parameters that were not used in producing loss. You can enable unused parameter detection by (1) passing the keyword argument find_unused_parameters=True to torch.nn.parallel.DistributedDataParallel; (2) making sure all forward function outputs participate in calculating loss. If you already have done the above two steps, then the distributed data parallel module wasn't able to locate the output tensors in the return value of your module's forward function. Please include the loss function and the structure of the return value of forward of your module when reporting this issue (e.g. list, dict, iterable).  File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/mmcv/parallel/distributed.py", line 42, in train_step
and self.reducer._rebuild_buckets()):

RuntimeError: Expected to have finished reduction in the prior iteration before starting a new one. This error indicates that your module has parameters that were not used in producing loss. You can enable unused parameter detection by (1) passing the keyword argument find_unused_parameters=True to torch.nn.parallel.DistributedDataParallel; (2) making sure all forward function outputs participate in calculating loss. If you already have done the above two steps, then the distributed data parallel module wasn't able to locate the output tensors in the return value of your module's forward function. Please include the loss function and the structure of the return value of forward of your module when reporting this issue (e.g. list, dict, iterable).
Killing subprocess 21215
Killing subprocess 21216
Killing subprocess 21217
Killing subprocess 21218
Traceback (most recent call last):
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/runpy.py", line 194, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/runpy.py", line 87, in _run_code
exec(code, run_globals)
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/torch/distributed/launch.py", line 340, in
main()
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/torch/distributed/launch.py", line 326, in main
sigkill_handler(signal.SIGTERM, None)  # not coming back
File "/export/lzh/anaconda3/envs/open-mmlab-four/lib/python3.8/site-packages/torch/distributed/launch.py", line 301, in sigkill_handler
raise subprocess.CalledProcessError(returncode=last_return_code, cmd=cmd)
subprocess.CalledProcessError: Command '['/export/lzh/anaconda3/envs/open-mmlab-four/bin/python', '-u', 'tools/train.py', '--local_rank=3', 'configs/tutorial/vgg19-finetune-cifar.py', '--launcher', 'pytorch']' returned non-zero exit status 1.

thank you for your report,
the PR https://github.com/open-mmlab/mmclassification/pull/634 is  about it, you can try it
we are still discussing what frozen_stages means

Sorry for the late reply.
I have reproduced the problem, and find that it's caused by the missing call of _freeze_stages in the initialization method.
Just add a line at the end of the __init__ method in SwinTransformer class:
self._freeze_stages()

We will fix it soon


Fixed button layout

Foxed the uneven button layout

i replaced the &nbsp hack with proper div and styling


[qwik-pod] Add content for Courses

Add Courses content for framework
Background
As a visitor of the framework specific Framework.dev site I would like to have the ability to see enough Courses content to keep me informed and find this site to be resourceful and useful.
Acceptance

[ ] Research a list of Courses
[ ] Add the Courses information in the data files
[ ] Test each individual look and external link

NOTE:

This task is timeboxed to a maximum of 4 hours.

I researched on the 19 of Oct and there seems to be no content for books yet, so I am moving this back to a later date.

Just invest 30 minutes to see if anything new was published

Spent a few minutes searching in Google if there is anything to share in the Courses tab but without luck, all of the things are presentations.

Add Misko’s JSDrops to this and let's move this to In Review.


Use incremental rebalance protocol

KIP-429 adds support for incremental rebalancing of consumer groups. What this means is that the rebalance events only carry the list of partitions that were actually newly-assigned or revoked.  If a currently-assigned partition does not change status during a rebalance, then the rebalance events will no longer carry that partition.
This allows us to significantly simplify the rebalance callback:

We no longer need to depend on revoke being called prior to every assign call.  This can be decoupled.
The list of revoked partitions are truly the revoked ones; we don't need to worry about them being added immediately after.
Similarly, we don't need to worry about assigned partitions already being active.  Any assigned partitions should always be new ones.

NOTE: Kafka doesn't support changing the type of a consumer group, so existing consumer groups using non-incremental rebalancing will need to be deleted and then recreated the new partition assignment strategy.

hello @james-johnston-thumbtack , looks very good to me.
Could you please rebase on main to be able to run the whole tests ? i have tried to fix some "unstable" tests.
Thanks

@fkarakas  It looks like the test suite passed now, but the overall process failed due to code coverage upload: "Please provide an upload token from codecov.io with valid arguments"

@james-johnston-thumbtack thank you 🙏


Failed at the<EMAIL_ADDRESS>test script 'cross-env BABEL_DISABLE_CACHE=1 NODE_ENV=test node --harmony_proxies node_modules/.bin/jest

Getting error on Node version 7.1.0
npm run test

><EMAIL_ADDRESS>test /Users/srikrishnagumma/Desktop/Projects/scalable-react-boilerplate
> cross-env BABEL_DISABLE_CACHE=1 NODE_ENV=test node --harmony_proxies node_modules/.bin/jest

node: bad option: --harmony_proxies

npm ERR! Darwin 16.1.0
npm ERR! argv "/usr/local/Cellar/node/7.1.0/bin/node" "/usr/local/bin/npm" "run" "test"
npm ERR! node v7.1.0
npm ERR! npm  v3.10.9
npm ERR! code ELIFECYCLE
npm ERR<EMAIL_ADDRESS>test: `cross-env BABEL_DISABLE_CACHE=1 NODE_ENV=test node --harmony_proxies node_modules/.bin/jest`
npm ERR! Exit status 9
npm ERR!
npm ERR! Failed at the<EMAIL_ADDRESS>test script 'cross-env BABEL_DISABLE_CACHE=1 NODE_ENV=test node --harmony_proxies node_modules/.bin/jest'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the scalable-react-boilerplate package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     cross-env BABEL_DISABLE_CACHE=1 NODE_ENV=test node --harmony_proxies node_modules/.bin/jest
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs scalable-react-boilerplate
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls scalable-react-boilerplate
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /Users/srikrishnagumma/Desktop/Projects/scalable-react-boilerplate/npm-debug.log

All my tests fail too.
I get another error though. All errors are of the form:
 FAIL  app/src/components/About/tests/index.test.js
  ● Test suite failed to run

    /Users/mdentremont/Projects/REACT/scalable-react-boilerplate/app/src/components/About/tests/index.test.js: Cannot find module '/Users/mdentremont/Projects/REACT/scalable-react-boilerplate/config/testing/webpack.test.config.js'

      at Function.Module._resolveFilename (module.js:469:15)
      at Function.Module._load (module.js:417:25)
      at Module.require (module.js:497:17)
      at require (internal/module.js:20:19)
      at localInteropRequire (node_modules/babel-plugin-webpack-loaders/lib/plugin.js:225:13)
      at PluginPass.CallExpression (node_modules/babel-plugin-webpack-loaders/lib/plugin.js:41:58)
      at newFn (node_modules/babel-traverse/lib/visitors.js:276:21)
      at NodePath._call (node_modules/babel-traverse/lib/path/context.js:76:18)
      at NodePath.call (node_modules/babel-traverse/lib/path/context.js:48:17)
      at NodePath.visit (node_modules/babel-traverse/lib/path/context.js:105:12)

Test Suites: 14 failed, 14 total
Tests:       0 total
Snapshots:   0 total
Time:        1.34s
Ran all test suites.


chore: Update build process to emit ESM modules and a UMD bundle

Implements same build process from @ngrx/db. See ngrx/db#8
Closes #210 #203 #180

Updated to address feedback and to upgrade to Angular 2 final ✨
Ping @robwormald

LGTM. Merge that ish and ship it.


hey

I am testing DAws, i d love to know how to get past the fake error generated?

post the admin password via a custom form.

To get past the fake error generated you need to login and to do that you need to send a post request holding the following post data: pass=DAws ("DAws" is the default password unless you changed it)
To send the post data you can do just like toby57 said (thanks for helping him out mate) or you can simply use a firefox add on like Live Http Headers, Temper Data or Hack Bar which will make life easier for you.


[enhancement]: Add TITLE to Invoke.bat

Is there an existing issue for this?

[X] I have searched the existing issues

Contact Details
<EMAIL_ADDRESS>
What should this feature add?
The batch file invokes an unidentified command terminal. How about adding the title?
e.g.,
@echo off title Invoke AI Session Controller PUSHD "%~dp0" setlocal ...
Then everyone knows why that random terminal is open and what it's running. You might only have one thing running, others of us may be operating on more complex desktops and have serval terminals open.
Alternatives
I titled my copy.
But the right thing to do is putting it into the production version so there's no UX gap to complain about ;-)
Additional Content
No response

Adding:
mode con: cols=150 lines=50
Would also be a nice touch. Most stock windows installs open at 120 lines. With these settings, no one will need to resize the terminal or wonder what its supposed to be doing.

Thanks for the suggestion! Would you be able to open a PR to add this?

Sorry, no, I'm not able to manage that.
On Tue, Sep 12, 2023 at 8:55 PM Millun Atluri @.***>
wrote:

Thanks for the suggestion! Would you be able to open a PR to add this?
—
Reply to this email directly, view it on GitHub
https://github.com/invoke-ai/InvokeAI/issues/4525#issuecomment-1716778379,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/BCPAIN2UBKB2YENMXKTA4CTX2EAADANCNFSM6AAAAAA4VQV5YQ
.
You are receiving this because you authored the thread.Message ID:
@.***>

I can write for you, though. I won't write in markdown for an SSG, but if
someone is willing to add the markdown for a feature write-up and do
the PR, I can help out. I write tech docs for a living.
On Wed, Sep 13, 2023 at 2:59 AM sdf asdf @.***> wrote:

Sorry, no, I'm not able to manage that.
On Tue, Sep 12, 2023 at 8:55 PM Millun Atluri @.***>
wrote:

Thanks for the suggestion! Would you be able to open a PR to add this?
—
Reply to this email directly, view it on GitHub
https://github.com/invoke-ai/InvokeAI/issues/4525#issuecomment-1716778379,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/BCPAIN2UBKB2YENMXKTA4CTX2EAADANCNFSM6AAAAAA4VQV5YQ
.
You are receiving this because you authored the thread.Message ID:
@.***>

That's a very generous offer - I'm happy to add any markdown necessary! A couple things come to mind:

Improved Developer Documentation <-- this would be great to get your feedback on the current state
How-To Guide for Unified Canvas

Are you in the Discord? Would love to chat more about this!

I'm willing to pass over an edit of pages you point me to if there are some you really hate. I don't understand yet enough to synthesize new feature guides as I'm new to AI and having enough headache figuring out the basics. I will start keeping some notes as I see gaps and rough bits to fix up on the weekends. At least until I can hire another writer, I'm working some long hours.

@cgwers following up here - my apologies, I totally missed this GitHub notification!
I would love your thoughts on how to improve the contributing documentation: https://invoke-ai.github.io/InvokeAI/contributing/CONTRIBUTING/
It feels messy and hard to get started with right now.


[DISCUSS] How to merge the two modules - dolphinscheduler-task-plugin and dolphinscheduler-datasource-plugin?

Search before asking

[X] I had searched in the issues and found no similar feature requirement.

Description
How to merge the two modules - dolphinscheduler-task-plugin and dolphinscheduler-datasource-plugin?
Related issues
#13448
Are you willing to submit a PR?

[X] Yes I am willing to submit a PR!

Code of Conduct

[X] I agree to follow this project's Code of Conduct

First question: After merging datasource-plugin and task-plugin, what should the name be? I feel like it can't be the name of a single party anymore, because there are both connected tasks and tasks running inside.
Second question: How to merge? Taking zeppelin as an example, I moved the zeppelin from datasource-plugin to task-plugin, and I feel that the code is already very streamlined, unable to remove redundant code. What's your suggestion?

Why you want to merge the task module and datasource module?

because they are all plugin.

Why you want to merge the task module and datasource module?

Because the data source center will be changed to a connection center, which will support non SQL connections such as zeppelin, it is necessary to merge the data source center module and task module. You can ask him for other details: @EricGao888.


Recursive Insert TEnitiy design time error

I have an entitiy STUDENT and it has a property GRADE. I want to insert firstly sub entities.
If I try recursive insert because GRADE can contains another sub entitiy too.
My insert function want TEntity, if I cast GRADE as TEntity, code try to assign GRADE as a STUDENT
how can I resolve this problem?
public int Insert(TEntity entity)
        {
var xx = entity.GetType().GetProperties().Where(x => x.PropertyType.FullName == item.SubPropName).FirstOrDefault();
                Type entityType = xx.PropertyType;
                    var insertableEntity = Activator.CreateInstance(entityType);
                    insertableEntity = cc;

                   var id = Insert((TEntity)insertableEntity);
                }

@aburcuburcu There isn't enough information here for us to be able to effectively investigate. Please post a small, runnable project/solution or complete code listing that demonstrates what you are trying to do and where it is not working.

EF Team Triage: Closing this issue as the requested additional details have not been provided and we have been unable to reproduce it.
BTW this is a canned response and may have info or details that do not directly apply to this particular issue. While we'd like to spend the time to uniquely address every incoming issue, we get a lot traffic on the EF projects and that is not practical. To ensure we maximize the time we have to work on fixing bugs, implementing new features, etc. we use canned responses for common triage decisions.