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

Bugfix/#43 fix controller restart on config change #114

Merged

Conversation

DivineThreepwood
Copy link
Member

@DivineThreepwood DivineThreepwood commented Jul 26, 2023

πŸ“œ Description

❗ Blocked by: openbase/jul#104

Code changes proposed during fixing issue #43 that is actually fixed in Jul PR openbase/jul#104

  • fix some linting issues
  • fix some generic class declarations.
  • improve registry shutdown.
  • Port UnitRemotePool to Koltin
  • Fix update issue of unit remote pool.

* #L%
*/

class CustomUnitPool : Manageable<Collection<Filter<UnitConfigType.UnitConfig>>> {

Check warning

Code scanning / detekt

Too many functions inside a/an file/class/object/interface always indicate a violation of the single responsibility principle. Maybe the file/class/object/interface wants to manage too many things at once. Extract functionality which clearly belongs together.

Class 'CustomUnitPool' with '14' functions detected. Defined threshold inside classes is set to '11'
*/

class CustomUnitPool : Manageable<Collection<Filter<UnitConfigType.UnitConfig>>> {
private val UNIT_REMOTE_REGISTRY_LOCK = ReentrantReadWriteLock()

Check warning

Code scanning / detekt

Variable names should follow the naming convention set in the projects configuration.

Private variable names should match the pattern: (_)?[a-z][A-Za-z0-9]*
@@ -49,7 +49,7 @@
private val BACKPRESSURE_STRATEGY = BackpressureStrategy.BUFFER

@Throws(BCOGraphQLError::class)
fun subscribeUnits(unitFilter: UnitFilter?): Publisher<UnitDataType.UnitData> {
fun subscribeUnits(unitFilter: UnitFilter): Publisher<UnitDataType.UnitData> {

Check warning

Code scanning / detekt

Restrict the number of throw statements in methods.

Too many throw statements in the function subscribeUnits. The maximum number of allowed throw statements is 2.
import org.openbase.bco.dal.lib.layer.unit.UnitRemote
import org.openbase.bco.registry.remote.Registries
import org.openbase.bco.registry.unit.lib.filter.UnitConfigFilterImpl
import org.openbase.jul.exception.*

Check warning

Code scanning / detekt

Wildcard imports should be replaced with imports using fully qualified class names. Wildcard imports can lead to naming conflicts. A library update can introduce naming clashes with your classes which results in compilation errors.

org.openbase.jul.exception.* is a wildcard import. Replace it with fully qualified imports.

class CustomUnitPool : Manageable<Collection<Filter<UnitConfigType.UnitConfig>>> {
private val UNIT_REMOTE_REGISTRY_LOCK = ReentrantReadWriteLock()
private var unitRegistryDataObserver: Observer<DataProvider<UnitRegistryDataType.UnitRegistryData>, UnitRegistryDataType.UnitRegistryData>

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
unitConfigDiff = ProtobufListDiff()
unitRemoteRegistry = RemoteControllerRegistry()
unitRegistryDataObserver =
Observer { source: DataProvider<UnitRegistryDataType.UnitRegistryData>, data: UnitRegistryDataType.UnitRegistryData? -> sync() }

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
* If the pool is already active, then the filter is directly applied.
* If you call this method twice, only the latest filter set is used.
*
* @param unitFilter this is a unit filter that can be used to limit the number of unit to observer. No filter means every unit is observed by this pool.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
* If the pool is already active, then the filter is directly applied.
* If you call this method twice, only the latest filter set is used.
*
* @param filters this set of filters can be used to limit the number of unit to observer. No filter means every unit is observed by this pool.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
* If the pool is already active, then the filter is directly applied.
* If you call this method twice, only the latest filter set is used.
*
* @param filters this set of filters can be used to limit the number of unit to observer. No filter means every unit is observed by this pool.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
Registries.getUnitRegistry().removeDataObserver(unitRegistryDataObserver)
} catch (ex: NotAvailableException) {
// if the registry is not available an observer deregistration is not required.
// This can for example be the case when the unit registry has already been terminated during the shutdown progress.

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style.

Line detected that is longer than the defined maximum line length in the code style.
Copy link
Member Author

@DivineThreepwood DivineThreepwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from my side everything is fine.

@DivineThreepwood DivineThreepwood requested a review from lhuxohl July 31, 2023 18:40
@DivineThreepwood DivineThreepwood added ready to review blocked PR is based on another PR and is therefore blocked until the other one has been merged. and removed changes requested labels Jul 31, 2023
lhuxohl
lhuxohl previously approved these changes Aug 1, 2023
lhuxohl
lhuxohl previously approved these changes Aug 3, 2023
@DivineThreepwood DivineThreepwood removed the blocked PR is based on another PR and is therefore blocked until the other one has been merged. label Aug 3, 2023
lhuxohl
lhuxohl previously approved these changes Aug 3, 2023
@DivineThreepwood DivineThreepwood merged commit d2a7cff into dev Aug 3, 2023
@DivineThreepwood DivineThreepwood deleted the bugfix/#43_fix_controller_restart_on_config_change branch August 3, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants