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

GraphQL crashes with AssertionError on custom fields with spaces in the name #464

Closed
jathanism opened this issue May 19, 2021 · 4 comments · Fixed by #821
Closed

GraphQL crashes with AssertionError on custom fields with spaces in the name #464

jathanism opened this issue May 19, 2021 · 4 comments · Fixed by #821
Assignees
Labels
type: bug Something isn't working as expected

Comments

@jathanism
Copy link
Contributor

jathanism commented May 19, 2021

Environment

  • Python version: 3.6+
  • Nautobot version: 1.0.1

Steps to Reproduce

  1. Create a custom field with a space in the name (e.g. "Support email") and bind it to an object (e.g. dcim.device)
  2. Populate the field value on a object (e.g. a device, with value "Support email = bob@example.com")
  3. Navigate to /graphql

Expected Behavior

The GraphQL interface should load, and there should be some way to use this field name in filtering. If custom field names with spaces are not valid, this should be disallowed. If they are valid, but not for GraphQL, this should be handled properly.

Observed Behavior

💥

Internal Server Error: /graphql/
Traceback (most recent call last):
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/sentry_sdk/integrations/django/views.py", line 67, in sentry_wrapped_callback
    return callback(request, *args, **kwargs)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/django/views/generic/base.py", line 63, in view
    self = cls(**initkwargs)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene_django/views.py", line 104, in __init__
    schema = graphene_settings.SCHEMA
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene_django/settings.py", line 127, in __getattr__
    val = perform_import(val, attr)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene_django/settings.py", line 66, in perform_import
    return import_from_string(val, setting_name)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene_django/settings.py", line 80, in import_from_string
    module = importlib.import_module(module_path)
  File "/Users/jathan/.pyenv/versions/3.8.7/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/Users/jathan/sandbox/src/nautobot/nautobot/core/graphql/schema_init.py", line 14, in <module>
    schema = graphene.Schema(query=Query, auto_camelcase=False)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene/types/schema.py", line 78, in __init__
    self.build_typemap()
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene/types/schema.py", line 167, in build_typemap
    self._type_map = TypeMap(
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene/types/typemap.py", line 80, in __init__
    super(TypeMap, self).__init__(types)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphql/type/typemap.py", line 31, in __init__
    self.update(reduce(self.reducer, types, OrderedDict()))  # type: ignore
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene/types/typemap.py", line 88, in reducer
    return self.graphene_reducer(map, type)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphene/types/typemap.py", line 117, in graphene_reducer
    return GraphQLTypeMap.reducer(map, internal_type)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphql/type/typemap.py", line 109, in reducer
    field_map = type_.fields
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphql/pyutils/cached_property.py", line 22, in __get__
    value = obj.__dict__[self.func.__name__] = self.func(obj)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphql/type/definition.py", line 198, in fields
    return define_field_map(self, self._fields)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphql/type/definition.py", line 234, in define_field_map
    assert_valid_name(arg_name)
  File "/Users/jathan/Library/Caches/pypoetry/virtualenvs/nautobot-b2ttWva6-py3.8/lib/python3.8/site-packages/graphql/utils/assert_valid_name.py", line 10, in assert_valid_name
    assert COMPILED_NAME_PATTERN.match(
AssertionError: Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "cf_Support email" does not.
@jathanism
Copy link
Contributor Author

IMO this is a bug. While the name field is a string, spaces should be disallowed so that it can be accurately treated as a slug-style machine-readable field. We might consider even changing it to a slug, and letting the label field do the work as the human-readable field with spaces in it.

@glennmatthews
Copy link
Contributor

Agree 100%. Migrating name to slug (and automatically slugifying the name of existing fields) is something we should definitely do in the next major release.

@ggidofalvy-tc
Copy link
Contributor

Do you folks happen to have an update on this?

@glennmatthews glennmatthews mentioned this issue Jul 26, 2021
2 tasks
@glennmatthews glennmatthews added the impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release label Aug 5, 2021
@glennmatthews
Copy link
Contributor

Sorry for the delay in responding here! Entirely replacing name with slug will be a breaking change and so will need to be deferred until a "2.0" Nautobot version. In the short term though it should be possible for us to at least do some fixup in GraphQL so that improper custom-field name values will at least not break it - I'm doing some investigation of this now.

@glennmatthews glennmatthews self-assigned this Aug 16, 2021
glennmatthews added a commit that referenced this issue Aug 17, 2021
* Fix #464: fixup custom field filter names

* Fixup misleading graphql warning message and docstrings; make vminterface verbose_name unique to avoid graphql collision
@glennmatthews glennmatthews removed the impact: breaking change This change or feature will remove or replace existing functionality; needs to be an X.0.0 release label Aug 17, 2021
glennmatthews added a commit that referenced this issue Aug 17, 2021
glennmatthews added a commit that referenced this issue Aug 20, 2021
* Create nautobot_database_ready signal and add an example to the dummy plugin and corresponding test case.

* Minor cleanup

* Restructure plugin development docs

* Add documentation for plugin usage of "nautobot_database_ready" signal

* Blacken

* Fix #464: fixup custom field filter names

* Fixup misleading graphql warning message and docstrings; make vminterface verbose_name unique to avoid graphql collision

* Test updates

* Fix typo
hellerve added a commit to hellerve/nautobot that referenced this issue Aug 23, 2021
* develop: (32 commits)
  Add object variable lookup (nautobot#837)
  Add release-note for nautobot#718
  Fix nautobot#718: Computed field template overflow (nautobot#831)
  Add release-notes for nautobot#825 and nautobot#832
  remove celery worker -B option (nautobot#827)
  Update Using Plugins guide to have Post Upgrade run. (nautobot#833)
  Add release-notes for nautobot#715 and nautobot#812
  GraphQL navbar overflowing into Graphiql  (nautobot#812)
  Add release-notes for nautobot#464, nautobot#731, nautobot#818
  Adding Logic for read-only hdb health check (nautobot#819)
  Fix nautobot#731 - enforce 'format' in config context schemas (nautobot#822)
  Fix GraphQL filterset handling of custom fields (nautobot#821)
  Add release-notes for nautobot#779, nautobot#791, nautobot#809
  Change the docker-compose file version to support start_period property (nautobot#810)
  Adding docs and logic to skip initialization in docker (nautobot#793)
  Fix incorrect tenancy display in related prefixes table (nautobot#806)
  Bump version to 1.1.3-beta.1
  Bump version and add release date
  Add release-notes for nautobot#785 and nautobot#786
  Prioritize LoganImporter over built-in importers. See nautobot#785 (nautobot#788)
  ...
hellerve added a commit to hellerve/nautobot that referenced this issue Aug 23, 2021
* develop: (32 commits)
  Add object variable lookup (nautobot#837)
  Add release-note for nautobot#718
  Fix nautobot#718: Computed field template overflow (nautobot#831)
  Add release-notes for nautobot#825 and nautobot#832
  remove celery worker -B option (nautobot#827)
  Update Using Plugins guide to have Post Upgrade run. (nautobot#833)
  Add release-notes for nautobot#715 and nautobot#812
  GraphQL navbar overflowing into Graphiql  (nautobot#812)
  Add release-notes for nautobot#464, nautobot#731, nautobot#818
  Adding Logic for read-only hdb health check (nautobot#819)
  Fix nautobot#731 - enforce 'format' in config context schemas (nautobot#822)
  Fix GraphQL filterset handling of custom fields (nautobot#821)
  Add release-notes for nautobot#779, nautobot#791, nautobot#809
  Change the docker-compose file version to support start_period property (nautobot#810)
  Adding docs and logic to skip initialization in docker (nautobot#793)
  Fix incorrect tenancy display in related prefixes table (nautobot#806)
  Bump version to 1.1.3-beta.1
  Bump version and add release date
  Add release-notes for nautobot#785 and nautobot#786
  Prioritize LoganImporter over built-in importers. See nautobot#785 (nautobot#788)
  ...
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants