You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ariatosca.apache.org by AviaE <gi...@git.apache.org> on 2017/05/08 18:29:57 UTC
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
GitHub user AviaE opened a pull request:
https://github.com/apache/incubator-ariatosca/pull/128
ARIA-210 Handle relative paths in CLI service-templates
This was a rather simple change, mainly involving adding absolute path
references.
The problems were only in `service-templates store` and in
`service-templates create-archive`.
`service-templates validate` was not affected.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-ariatosca ARIA-210-handle-relative-paths-in-cli-service-templates
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-ariatosca/pull/128.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #128
----
commit 0a3ea7991d6218438318d93232f82e7d7d42d4ae
Author: Avia Efrat <av...@gigaspaces.com>
Date: 2017-05-08T14:45:23Z
ARIA-210 Handle relative paths in CLI service-templates
This was a rather simple change, mainly involving adding absolute path
references.
The problems were only in `service-templates store` and in
`service-templates create-archive`.
`service-templates validate` was not affected.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by AviaE <gi...@git.apache.org>.
Github user AviaE commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115502906
--- Diff: tests/cli/test_service_templates.py ---
@@ -244,3 +258,13 @@ def test_create_archive_successful(self, monkeypatch, mock_object):
monkeypatch.setattr(csar, 'write', mock_object)
self.invoke('service_templates create_archive stubpath stubdest')
assert 'CSAR archive created at stubdest' in self.logger_output_string
+
+ def test_create_archive_from_relative_path(self, monkeypatch, mock_object):
+
+ monkeypatch.setattr(os.path, 'isfile', lambda x: True)
+ monkeypatch.setattr(os.path, 'isfile', mock_object)
--- End diff --
Indeed. So I removed that line.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by AviaE <gi...@git.apache.org>.
Github user AviaE commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115501254
--- Diff: aria/cli/service_template_utils.py ---
@@ -53,7 +53,7 @@ def get(source, service_template_filename):
return _get_service_template_file_from_archive(source, service_template_filename)
--- End diff --
Added tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115324450
--- Diff: aria/cli/commands/service_templates.py ---
@@ -195,7 +195,9 @@ def create_archive(service_template_path, destination, logger):
`destination` is the path of the output CSAR archive file
"""
logger.info('Creating a CSAR archive')
- csar.write(os.path.dirname(service_template_path), service_template_path, destination, logger)
+ if not destination.endswith('.csar'):
--- End diff --
maybe extract `.csar` suffix to a constant in csar.py? seems like its defined in several places now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-ariatosca/pull/128
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115325816
--- Diff: aria/cli/service_template_utils.py ---
@@ -53,7 +53,7 @@ def get(source, service_template_filename):
return _get_service_template_file_from_archive(source, service_template_filename)
--- End diff --
What about adding a short test or two for these scenarios? :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115498975
--- Diff: aria/cli/csar.py ---
@@ -22,7 +22,7 @@
import requests
from ruamel import yaml
-
+CSAR_FILE_EXTENSION = '.csar'
--- End diff --
do a quick search for `.csar`, there should be at least one more reference to it in modules you've touched anyway :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115498822
--- Diff: tests/cli/test_service_templates.py ---
@@ -244,3 +258,13 @@ def test_create_archive_successful(self, monkeypatch, mock_object):
monkeypatch.setattr(csar, 'write', mock_object)
self.invoke('service_templates create_archive stubpath stubdest')
assert 'CSAR archive created at stubdest' in self.logger_output_string
+
+ def test_create_archive_from_relative_path(self, monkeypatch, mock_object):
+
+ monkeypatch.setattr(os.path, 'isfile', lambda x: True)
+ monkeypatch.setattr(os.path, 'isfile', mock_object)
--- End diff --
doesnt this line override the previous one?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115324345
--- Diff: aria/cli/csar.py ---
@@ -38,17 +38,19 @@
}
-def write(source, entry, destination, logger):
- source = os.path.expanduser(source)
- destination = os.path.expanduser(destination)
- entry_definitions = os.path.join(source, entry)
+def write(service_template_path, destination, logger):
--- End diff --
good, i like the signature better this way
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by ran-z <gi...@git.apache.org>.
Github user ran-z commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115499156
--- Diff: tests/cli/test_service_templates.py ---
@@ -131,6 +133,18 @@ def test_store_no_exception(self, monkeypatch, mock_object):
assert 'Service template {name} stored'.format(
name=mock_models.SERVICE_TEMPLATE_NAME) in self.logger_output_string
+ def test_store_relative_path_single_yaml_file(self, monkeypatch, mock_object):
+ monkeypatch.setattr(Core, 'create_service_template', mock_object)
+ monkeypatch.setattr(os.path, 'isfile', lambda x: True)
+ monkeypatch.setattr(service_template_utils, '_is_archive', lambda x: False)
+
+ self.invoke('service_templates store service_template.yaml {name}'.format(
+ name=mock_models.SERVICE_TEMPLATE_NAME))
+
+ mock_object.assert_called_with(os.path.join(os.getcwd(), 'service_template.yaml'),
--- End diff --
just curious why you've decided to use `join` and `getcwd` here rather than absolute path (like you did in the actual code). not that it should matter :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by AviaE <gi...@git.apache.org>.
Github user AviaE commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115502106
--- Diff: tests/cli/test_service_templates.py ---
@@ -131,6 +133,18 @@ def test_store_no_exception(self, monkeypatch, mock_object):
assert 'Service template {name} stored'.format(
name=mock_models.SERVICE_TEMPLATE_NAME) in self.logger_output_string
+ def test_store_relative_path_single_yaml_file(self, monkeypatch, mock_object):
+ monkeypatch.setattr(Core, 'create_service_template', mock_object)
+ monkeypatch.setattr(os.path, 'isfile', lambda x: True)
+ monkeypatch.setattr(service_template_utils, '_is_archive', lambda x: False)
+
+ self.invoke('service_templates store service_template.yaml {name}'.format(
+ name=mock_models.SERVICE_TEMPLATE_NAME))
+
+ mock_object.assert_called_with(os.path.join(os.getcwd(), 'service_template.yaml'),
--- End diff --
Nothing serious, just thought that it demonstrates the fact that we are expecting the paths to be relative to the current working directory.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-ariatosca pull request #128: ARIA-210 Handle relative paths in CLI...
Posted by AviaE <gi...@git.apache.org>.
Github user AviaE commented on a diff in the pull request:
https://github.com/apache/incubator-ariatosca/pull/128#discussion_r115501629
--- Diff: aria/cli/csar.py ---
@@ -22,7 +22,7 @@
import requests
from ruamel import yaml
-
+CSAR_FILE_EXTENSION = '.csar'
--- End diff --
I did before, these were the only places =)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---