Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some questions about StorageUrl and Mutex in Connection struct #142

Open
Halfi opened this issue Mar 19, 2019 · 4 comments
Open

Some questions about StorageUrl and Mutex in Connection struct #142

Halfi opened this issue Mar 19, 2019 · 4 comments

Comments

@Halfi
Copy link

Halfi commented Mar 19, 2019

Why don't use sync.RWMutex and RLock in method Connection.storage for read StorageUrl?
Why OnReAuth don't lock mutex in this case?

Why variable Connection.StorageUrl is exportable and sensitive to sync and you don't have exportable getters and setters for this variable with correct locks?

@ncw
Copy link
Owner

ncw commented Mar 19, 2019

Why don't use sync.RWMutex and RLock in method Connection.storage for read StorageUrl?

We could do - not a bad plan!

Why OnReAuth don't lock mutex in this case?

The lock will be held during a re-authentication

Why variable Connection.StorageUrl is exportable and sensitive to sync and you don't have exportable getters and setters for this variable with correct locks?

I've tried to keep the library backwards compatible which is why there are no setters/getters.

There perhaps should be Getters for StorageURL - I'm not sure it needs a setter as if you are setting it it is likely to be just at the start.

@Halfi
Copy link
Author

Halfi commented Mar 19, 2019

I've tried to keep the library backwards compatible which is why there are no setters/getters.

But getter don't break backwards compatible. It's looks like new feature. It can be locked with rwmutex and work well.

There perhaps should be Getters for StorageURL - I'm not sure it needs a setter as if you are setting it it is likely to be just at the start.

Yes, setter shouldn't be.

@Halfi
Copy link
Author

Halfi commented Mar 19, 2019

I have to do some custom request. My swift provider support bulk deletion only in post method. And I have been copying a lot of code (and got some potential problem with sync reading StorageUrl) only for change a few words in method :-)
Maybe think about public methods for parsing standard swift response? I have copied parseResponseStatus, drainAndClose, readJson as is and some error variables and types for correct work that methods.

@ncw
Copy link
Owner

ncw commented Apr 10, 2019

Do you want to send a PR to make some of those things public?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants