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

[Suggestion] Giving instances ID's #477

Closed
sarpaykent opened this issue Jun 22, 2017 · 7 comments
Closed

[Suggestion] Giving instances ID's #477

sarpaykent opened this issue Jun 22, 2017 · 7 comments

Comments

@sarpaykent
Copy link

Hello, I don't have detailed knowledge about the whole system but why don't we give instances ID's (optionally) so getting instances will be easier and faster than ever

Thanks,
Sarp

@Geolim4
Copy link
Member

Geolim4 commented Jun 22, 2017

Hello,

What are you talking about exactly ?

@sarpaykent
Copy link
Author

sarpaykent commented Jun 22, 2017

Well, correct me if I'm wrong but currently when we use getInstance('driverName') It searches for current drivers with configuration. I'm suggesting that when we are creating drivers lets say

new Psr16Adapter($defaultDriver, $driverID, [/* Config array */]);

and when getting a driver

getInstance($driverID)

This way there is no problem with getting drivers btw I still don't understand how we can create 2 file drivers and get them without knowing all the configuration they have. (For example file drive 1 path= /path1/, file drive 2 path = /path2/)

Reference: https://github.com/PHPSocialNetwork/phpfastcache/wiki/%5BV5%5D-Why-calling-getInstance()-each-time-is-a-bad-practice-%3F

(One extra: In wiki page, there isn't information about all the configs easiest example is the path. It could be the only one that is missing but wanted to say that too https://github.com/PHPSocialNetwork/phpfastcache/wiki/%5BV4%CB%96%5D-The-driver-options )


@Geolim4
Copy link
Member

Geolim4 commented Jun 22, 2017

Internally PhpFastCache already make use of an instance ID based on the driver & config:
https://github.com/PHPSocialNetwork/phpfastcache/blob/final/src/phpFastCache/CacheManager.php#L205

Currently I can't break the compatibility by adding a new param to Psr16Adapter class as PhpFastCache follows the semver specs.

However we can see things on a different way:

  • Adding a getter (only) getInstanceId() to ExtendedCacheItemPoolInterface which represents the current (unique) instance ID.
  • Adding an option to $config to set the Instance ID instead of generating it automatically.
  • Throwing an Exception if trying to setup an already existing instance ID
  • Adding a getter to CacheManager: CacheManager::getInstanceById($id)

What do you think about that concept ?

--
This change would be targeted for 6.1.x and 7.0 to be semver compliant
The 6.1 will be in development until October and will fast forward the 6.0 bug-fixes/improvements.

@Geolim4 Geolim4 self-assigned this Jun 22, 2017
@Geolim4 Geolim4 added this to the 6.1.0 milestone Jun 22, 2017
@Geolim4 Geolim4 added this to Todo in V6.1 Roadmap Jun 22, 2017
@Geolim4 Geolim4 added this to Todo in V7 Roadmap Jun 22, 2017
@sarpaykent
Copy link
Author

This could be great!

Thanks @Geolim4

@Geolim4
Copy link
Member

Geolim4 commented Jun 23, 2017

Okay the roadmap has been updated.
I'm closing this issue, you will be notified by this one when the card will change it's state:
https://github.com/PHPSocialNetwork/phpfastcache/projects/4#card-3472014

Cheers,
Georges

@Geolim4 Geolim4 closed this as completed Jun 23, 2017
@Geolim4 Geolim4 moved this from Todo to In progress in V6.1 Roadmap Oct 13, 2017
@Geolim4 Geolim4 removed this from In progress in V6.1 Roadmap Oct 17, 2017
@Geolim4
Copy link
Member

Geolim4 commented Oct 17, 2017

Will be done in V7 instead of 6.1 as planned initially.
V7 is planned to be out fo Summer 2018, see timeline

@Geolim4
Copy link
Member

Geolim4 commented Oct 17, 2017

Hello @HackHers this feature is now testable on V7 if you want to.

Cheers,
Georges.L

@Geolim4 Geolim4 modified the milestones: 6.1.0, 7.0.0 Oct 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
V7 Roadmap
  
Done
Development

No branches or pull requests

2 participants