Skip to content

Conversation

@ilu2112
Copy link
Contributor

@ilu2112 ilu2112 commented Jul 6, 2016

I tried to create Config model and add 1 to 1 relation between Instance and newly created Config model but I don't know how to do it using existing fields. Anyways, I think that if snippet's global config is really global then it should come with directly in JSON with Instance model.

@opalczynski Please take a look here :)

connection = self._get_connection()
return connection.request(http_method, endpoint)['config']

def set_config(self, config):
Copy link
Contributor

@opalczynski opalczynski Jul 7, 2016

Choose a reason for hiding this comment

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

Maybe check here if this is a valid dict:

        if isinstance(config, six.string_types):
            try:
                config = json.loads(config)
            except (ValueError, TypeError):
                raise SyncanoValueError('...')

        if not isinstance(config, dict):
            raise SyncanoValueError('...')

@opalczynski
Copy link
Contributor

One small comment left;

LGTM.

from tests.integration_test import InstanceMixin, IntegrationTest


class SnippetConfigTest(InstanceMixin, IntegrationTest):
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a test here which check something like this:

  1. Create config with some field
  2. Update the config with new field
  3. Check if some field is still present in config after update.

@opalczynski
Copy link
Contributor

PLS merge.

@ilu2112 ilu2112 merged commit f2d77ce into develop Jul 9, 2016
@opalczynski opalczynski deleted the LIB-800 branch July 19, 2016 13:15
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

Successfully merging this pull request may close these issues.

3 participants