cwtch.im
/
cwtch
16
20
Fork 16
Code Issues 10 Pull Requests 2 Releases 163 Wiki Activity

Upgrade Dependencies + Clean up Groups #350

Merged
dan merged 5 commits from upgrade into master 2021-05-04 20:24:21 +00:00
Conversation 6 Commits 5 Files Changed 22 +135 -245
sarah commented 2021-05-03 21:30:33 +00:00
Owner
Copy Link
No description provided.
dan was assigned by sarah 2021-05-03 21:30:33 +00:00
sarah changed title from Upgrade Dependencies + Disable Inline Invites pending UI Design to Upgrade Dependencies + Clean up Groups 2021-05-03 23:35:10 +00:00
buildbot commented 2021-05-03 23:35:48 +00:00
Member
Copy Link

Drone Build Status: failure

https://build.openprivacy.ca/cwtch.im/cwtch/77

Drone Build Status: failure https://build.openprivacy.ca/cwtch.im/cwtch/77
buildbot commented 2021-05-03 23:37:30 +00:00
Member
Copy Link

Drone Build Status: failure

https://build.openprivacy.ca/cwtch.im/cwtch/79

Drone Build Status: failure https://build.openprivacy.ca/cwtch.im/cwtch/79
buildbot commented 2021-05-03 23:46:50 +00:00
Member
Copy Link

Drone Build Status: success

https://build.openprivacy.ca/cwtch.im/cwtch/81

Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/81
buildbot commented 2021-05-03 23:46:58 +00:00
Member
Copy Link

Drone Build Status: success

https://build.openprivacy.ca/cwtch.im/cwtch/83

Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/83
dan requested changes 2021-05-04 19:07:02 +00:00
app/utils/utils.go
@ -12,9 +13,11 @@ import (
// may fill that usecase better
func WaitGetPeer(app app2.Application, name string) peer.CwtchPeer {
dan commented 2021-05-04 19:02:49 +00:00
Owner
Copy Link

this function... seems.. not great, and is only used by app/servermon? do we just want to remove that?

this function... seems.. not great, and is only used by app/servermon? do we just want to remove that?
sarah commented 2021-05-04 19:17:30 +00:00
Author
Owner
Copy Link

See below for my comment on App/Name and Profile.

See below for my comment on App/Name and Profile.
dan marked this conversation as resolved
peer/cwtch_peer.go Outdated
@ -467,4 +434,3 @@
group := cp.Profile.GetGroup(groupid)
defer cp.mutex.Unlock()
if group == nil {
return errors.New("invalid group id")
dan commented 2021-05-04 18:59:44 +00:00
Owner
Copy Link

need an unlock before return

need an unlock before return
sarah commented 2021-05-04 19:09:47 +00:00
Author
Owner
Copy Link

There is an unlock 2 lines down...

There is an unlock 2 lines down...
dan commented 2021-05-04 19:18:55 +00:00
Owner
Copy Link

not inside the if group == nil { return

that error would leave it locked no?

not inside the if group == nil { return that error would leave it locked no?
sarah commented 2021-05-04 19:21:51 +00:00
Author
Owner
Copy Link

Ah good catch!

Ah good catch!
peer/cwtch_peer.go
@ -617,1 +576,4 @@
}
if key == attr.GetLocalScope("name") {
return cp.Profile.Name, true
dan commented 2021-05-04 19:00:40 +00:00
Owner
Copy Link

wait, why this? why not using the attribute map like all others?

do you have a paired override for setAttribute? which also we prolly shouldn't do? I thougt we were just gonna delete Profile.Name

wait, why this? why not using the attribute map like all others? do you have a paired override for setAttribute? which also we prolly shouldn't do? I thougt we were just gonna delete Profile.Name
sarah commented 2021-05-04 19:16:38 +00:00
Author
Owner
Copy Link

this code will use default to set attribute first..this is a fallback for the case where we create a profile (which is prior to InitForEvents) and then immediately try to look it up (like in the integration test)...we don't yet have any way to alert the storage engine to save name as an attribute.

At some point we should reconcile this - probably by removing Name from profile, and any reference in App(Client/Server) - but that is a bigger change that touches a lot more of the integration test and app so I felt this was a good compromise that removes the troublesome APIs while preserving the speciality of Name until we have the time to redo Application to not rely on it.

this code will use default to set attribute first..this is a fallback for the case where we create a profile (which is prior to InitForEvents) and then immediately try to look it up (like in the integration test)...we don't yet have any way to alert the storage engine to save name as an attribute. At some point we should reconcile this - probably by removing Name from profile, and any reference in App(Client/Server) - but that is a bigger change that touches a lot more of the integration test and app so I felt this was a good compromise that removes the troublesome APIs while preserving the speciality of Name until we have the time to redo Application to not rely on it.
dan commented 2021-05-04 19:23:36 +00:00
Owner
Copy Link

ah i see, I missed its a fall through if we didnt get the attribute which we check first!
yeah the UI has all that code around name getting and falling back to onion but yeah putting it here is prolly wrong. ok thats fine

ah i see, I missed its a fall through if we didnt get the attribute which we check first! yeah the UI has all that code around name getting and falling back to onion but yeah putting it here is prolly wrong. ok thats fine
dan marked this conversation as resolved
buildbot commented 2021-05-04 19:07:40 +00:00
Member
Copy Link

Drone Build Status: success

https://build.openprivacy.ca/cwtch.im/cwtch/85

Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/85
buildbot commented 2021-05-04 19:30:37 +00:00
Member
Copy Link

Drone Build Status: success

https://build.openprivacy.ca/cwtch.im/cwtch/87

Drone Build Status: success https://build.openprivacy.ca/cwtch.im/cwtch/87
dan merged commit deea608da6 into master 2021-05-04 20:24:20 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
dan
Labels
Clear labels   
applications

   
BLOCKED

   
bug

   
design

   
duplicate

   
enhancement

   
fixed?

   
funding-needed

   
help wanted

   
infrastructure

   
invalid

   
payments

   
qubes

   
question

   
ready-for-implementation

   
refactor

   
spam

   
tapir-server

   
testing

   
tor

   
wontfix
No Label
applications
BLOCKED
bug
design
duplicate
enhancement
fixed?
funding-needed
help wanted
infrastructure
invalid
payments
qubes
question
ready-for-implementation
refactor
spam
tapir-server
testing
tor
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
dan
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cwtch.im/cwtch#350
No description provided.
Delete Branch "upgrade"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?