You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/07/29 17:40:32 UTC

[GitHub] [ignite] ivandasch commented on a change in pull request #8098: Ducktape codestyle

ivandasch commented on a change in pull request #8098:
URL: https://github.com/apache/ignite/pull/8098#discussion_r462420728



##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_path.py
##########
@@ -13,44 +13,55 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""
+This module contains ignite path resolve utilities.
+"""
+
 import os
 
 from ignitetest.tests.utils.version import get_version, IgniteVersion
 
-"""
-This module provides Ignite path methods
-"""
-
 
 class IgnitePath:
-    SCRATCH_ROOT = "/mnt"
-    IGNITE_INSTALL_ROOT = "/opt"
-
     """Path resolver for Ignite system tests which assumes the following layout:
 
-        /opt/ignite-dev          # Current version of Ignite under test
-        /opt/ignite-2.7.6        # Example of an older version of Ignite installed from tarball
-        /opt/ignite-<version>    # Other previous versions of Ignite
-        ...
-    """
+       /opt/ignite-dev          # Current version of Ignite under test
+       /opt/ignite-2.7.6        # Example of an older version of Ignite installed from tarball
+       /opt/ignite-<version>    # Other previous versions of Ignite
+       ...
+   """
+    SCRATCH_ROOT = "/mnt"
+    IGNITE_INSTALL_ROOT = "/opt"
 
     def __init__(self, context):
         self.project = context.globals.get("project", "ignite")
 
     def home(self, node_or_version, project=None):
-        version = self._version(node_or_version)
+        """
+        :param node_or_version: Ignite service node or IgniteVersion instance.
+        :param project: Project name.
+        :return: Home directory.
+        """
+        version = self.__version__(node_or_version)
         home_dir = project or self.project
         if version is not None:
             home_dir += "-%s" % str(version)
 
         return os.path.join(IgnitePath.IGNITE_INSTALL_ROOT, home_dir)
 
     def script(self, script_name, node_or_version, project=None):
-        version = self._version(node_or_version)
+        """
+        :param script_name: Script name.
+        :param node_or_version: Ignite service node or IgniteVersion instance.
+        :param project: Project name.
+        :return: Full path to script.
+        """
+        version = self.__version__(node_or_version)
         return os.path.join(self.home(version, project=project), "bin", script_name)
 
-    def _version(self, node_or_version):
+    @staticmethod

Review comment:
       Both pycharm and pylint generate warnings on it. 

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_path.py
##########
@@ -13,44 +13,55 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""
+This module contains ignite path resolve utilities.
+"""
+
 import os
 
 from ignitetest.tests.utils.version import get_version, IgniteVersion
 
-"""
-This module provides Ignite path methods
-"""
-
 
 class IgnitePath:
-    SCRATCH_ROOT = "/mnt"
-    IGNITE_INSTALL_ROOT = "/opt"
-
     """Path resolver for Ignite system tests which assumes the following layout:
 
-        /opt/ignite-dev          # Current version of Ignite under test
-        /opt/ignite-2.7.6        # Example of an older version of Ignite installed from tarball
-        /opt/ignite-<version>    # Other previous versions of Ignite
-        ...
-    """
+       /opt/ignite-dev          # Current version of Ignite under test
+       /opt/ignite-2.7.6        # Example of an older version of Ignite installed from tarball
+       /opt/ignite-<version>    # Other previous versions of Ignite
+       ...
+   """
+    SCRATCH_ROOT = "/mnt"
+    IGNITE_INSTALL_ROOT = "/opt"
 
     def __init__(self, context):
         self.project = context.globals.get("project", "ignite")
 
     def home(self, node_or_version, project=None):
-        version = self._version(node_or_version)
+        """
+        :param node_or_version: Ignite service node or IgniteVersion instance.
+        :param project: Project name.
+        :return: Home directory.
+        """
+        version = self.__version__(node_or_version)
         home_dir = project or self.project
         if version is not None:
             home_dir += "-%s" % str(version)
 
         return os.path.join(IgnitePath.IGNITE_INSTALL_ROOT, home_dir)
 
     def script(self, script_name, node_or_version, project=None):
-        version = self._version(node_or_version)
+        """
+        :param script_name: Script name.
+        :param node_or_version: Ignite service node or IgniteVersion instance.
+        :param project: Project name.
+        :return: Full path to script.
+        """
+        version = self.__version__(node_or_version)
         return os.path.join(self.home(version, project=project), "bin", script_name)
 
-    def _version(self, node_or_version):
+    @staticmethod
+    def __version__(node_or_version):

Review comment:
       Yep, you are right

##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_path.py
##########
@@ -13,44 +13,55 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""
+This module contains ignite path resolve utilities.
+"""
+
 import os
 
 from ignitetest.tests.utils.version import get_version, IgniteVersion
 
-"""
-This module provides Ignite path methods
-"""
-
 
 class IgnitePath:
-    SCRATCH_ROOT = "/mnt"
-    IGNITE_INSTALL_ROOT = "/opt"
-
     """Path resolver for Ignite system tests which assumes the following layout:
 
-        /opt/ignite-dev          # Current version of Ignite under test
-        /opt/ignite-2.7.6        # Example of an older version of Ignite installed from tarball
-        /opt/ignite-<version>    # Other previous versions of Ignite
-        ...
-    """
+       /opt/ignite-dev          # Current version of Ignite under test
+       /opt/ignite-2.7.6        # Example of an older version of Ignite installed from tarball
+       /opt/ignite-<version>    # Other previous versions of Ignite
+       ...
+   """
+    SCRATCH_ROOT = "/mnt"
+    IGNITE_INSTALL_ROOT = "/opt"
 
     def __init__(self, context):
         self.project = context.globals.get("project", "ignite")
 
     def home(self, node_or_version, project=None):
-        version = self._version(node_or_version)
+        """
+        :param node_or_version: Ignite service node or IgniteVersion instance.
+        :param project: Project name.
+        :return: Home directory.
+        """
+        version = self.__version__(node_or_version)
         home_dir = project or self.project
         if version is not None:
             home_dir += "-%s" % str(version)
 
         return os.path.join(IgnitePath.IGNITE_INSTALL_ROOT, home_dir)
 
     def script(self, script_name, node_or_version, project=None):
-        version = self._version(node_or_version)
+        """
+        :param script_name: Script name.
+        :param node_or_version: Ignite service node or IgniteVersion instance.
+        :param project: Project name.
+        :return: Full path to script.
+        """
+        version = self.__version__(node_or_version)
         return os.path.join(self.home(version, project=project), "bin", script_name)
 
-    def _version(self, node_or_version):
+    @staticmethod
+    def __version__(node_or_version):

Review comment:
       Yep, you are right




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org