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

Feature/status board #708

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

cardene
Copy link

@cardene cardene commented Aug 8, 2017

No description provided.

from share.models import ProviderRegistration, SiteBanner, CeleryTaskResult, HarvestLog, SourceConfig


from share.models import ProviderRegistration, SiteBanner, CeleryTaskResult, logs
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove all the from share.models imports, just use from share import models (and update existing things to match)

api/urls.py Outdated
@@ -86,6 +88,8 @@ def register_url(self, subclass, viewset):
register_route(r'rawdata', views.RawDatumViewSet)
register_route(r'user', views.ShareUserViewSet)
register_route(r'sources', views.SourceViewSet)
register_route(r'HarvestLog', views.HarvestLogViewSet)
register_route(r'SourceConfig', views.SourceConfigViewSet)
Copy link
Contributor

Choose a reason for hiding this comment

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

these routes should be snake case and plural (harvest_logs, source_configs)

@@ -5,3 +5,5 @@
from .registration import * # noqa
from .schema import * # noqa
from .banner import * # noqa
from .harvestlogs import * # noqa
from .sourceConfig import * # noqa
Copy link
Contributor

Choose a reason for hiding this comment

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

rename these files:
harvestlogs.py => harvest_logs.py
sourceConfig.py => source_config.py

from share.util import IDObfuscator

from api.serializers import HarvestLogSerializer
from share.models import HarvestLog
Copy link
Contributor

Choose a reason for hiding this comment

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

import ordering from most broadly available to most specific:

  • built-ins
  • 3rd-party imports
  • 2nd-party imports
  • 1st-party imports

from share.models import HarvestLog

class SourceConfigFilterBackend(MultipleChoiceFilter):
def filter_queryset(self, request, queryset, view, conjoined=True):
Copy link
Contributor

Choose a reason for hiding this comment

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

remove conjoined

@@ -28,13 +28,15 @@ class ShareObjectViewSet(viewsets.ReadOnlyModelViewSet):

# Override get_queryset to handle items marked as deleted.
def get_queryset(self, list=True):
# import ipdb; ipdb.set_trace()
Copy link
Contributor

Choose a reason for hiding this comment

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

delete

queryset = super().get_queryset()
if list and hasattr(queryset.model, 'is_deleted'):
return queryset.exclude(is_deleted=True)
return queryset

# Override to convert encoded pk to an actual pk
def get_object(self):
import ipdb; ipdb.set_trace()
Copy link
Contributor

Choose a reason for hiding this comment

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

delete delete

@@ -20,8 +20,8 @@
from api.pagination import CursorPagination
from api.authentication import APIV1TokenBackPortAuthentication
from api.permissions import ReadOnlyOrTokenHasScopeOrIsAuthenticated
from api.serializers import FullNormalizedDataSerializer, BasicNormalizedDataSerializer, \
RawDatumSerializer, ShareUserSerializer, SourceSerializer
from api.serializers import FullNormalizedDataSerializer, BasicNormalizedDataSerializer,RawDatumSerializer, ShareUserSerializer, SourceSerializer
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 space!

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