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.

Sign up for GitHub

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

fix: move manual code to logging_v2 #83

Merged
merged 8 commits into from
Oct 30, 2020
Merged

fix: move manual code to logging_v2 #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
91368c4
chore: move manual code to logging_v2
busunkim96 Oct 29, 2020
5cdf501
test: fix import
busunkim96 Oct 29, 2020
0c2e154
fix: fix docs and lint
busunkim96 Oct 29, 2020
b1e7cce
fix: fix availability of __version__, Client
busunkim96 Oct 29, 2020
5231d95
fix: synchronize logging and logging_v2
busunkim96 Oct 30, 2020
1aa0294
fix: remove generated clients from __all__
busunkim96 Oct 30, 2020
fc7c231
test: fix system test
busunkim96 Oct 30, 2020
046d787
chore: blacken
busunkim96 Oct 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/client.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Cloud Logging Client
==========================

.. automodule:: google.cloud.logging.client
.. automodule:: google.cloud.logging_v2.client
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/entries.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Entries
=======

.. automodule:: google.cloud.logging.entries
.. automodule:: google.cloud.logging_v2.entries
:members:
:show-inheritance:
:member-order: groupwise
2 changes: 1 addition & 1 deletion docs/handlers-app-engine.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Google App Engine flexible Log Handler
======================================

.. automodule:: google.cloud.logging.handlers.app_engine
.. automodule:: google.cloud.logging_v2.handlers.app_engine
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/handlers-container-engine.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Google Kubernetes Engine Log Handler
====================================

.. automodule:: google.cloud.logging.handlers.container_engine
.. automodule:: google.cloud.logging_v2.handlers.container_engine
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/handlers.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Python Logging Module Handler
==============================

.. automodule:: google.cloud.logging.handlers.handlers
.. automodule:: google.cloud.logging_v2.handlers.handlers
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/logger.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Logger
======

.. automodule:: google.cloud.logging.logger
.. automodule:: google.cloud.logging_v2.logger
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/metric.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Metrics
=======

.. automodule:: google.cloud.logging.metric
.. automodule:: google.cloud.logging_v2.metric
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/sink.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Sinks
=====

.. automodule:: google.cloud.logging.sink
.. automodule:: google.cloud.logging_v2.sink
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/transports-base.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Python Logging Handler Sync Transport
======================================

.. automodule:: google.cloud.logging.handlers.transports.base
.. automodule:: google.cloud.logging_v2.handlers.transports.base
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/transports-sync.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Python Logging Handler Sync Transport
======================================

.. automodule:: google.cloud.logging.handlers.transports.sync
.. automodule:: google.cloud.logging_v2.handlers.transports.sync
:members:
:show-inheritance:
2 changes: 1 addition & 1 deletion docs/transports-thread.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ Python Logging Handler Threaded Transport
=========================================


.. automodule:: google.cloud.logging.handlers.transports.background_thread
.. automodule:: google.cloud.logging_v2.handlers.transports.background_thread
:members:
:show-inheritance:
52 changes: 36 additions & 16 deletions google/cloud/logging/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,40 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

"""Google Stackdriver Logging API wrapper."""


from pkg_resources import get_distribution

__version__ = get_distribution("google-cloud-logging").version

from google.cloud.logging.client import Client


ASCENDING = "timestamp asc"
"""Query string to order by ascending timestamps."""
DESCENDING = "timestamp desc"
"""Query string to order by decending timestamps."""

__all__ = ["__version__", "ASCENDING", "Client", "DESCENDING"]
from google.cloud.logging_v2 import __version__
from google.cloud.logging_v2 import ASCENDING
from google.cloud.logging_v2 import DESCENDING
from google.cloud.logging_v2.client import Client
from google.cloud.logging_v2.entries import logger_name_from_path
from google.cloud.logging_v2.entries import LogEntry
from google.cloud.logging_v2.entries import TextEntry
from google.cloud.logging_v2.entries import StructEntry
from google.cloud.logging_v2.entries import ProtobufEntry
from google.cloud.logging_v2 import handlers
from google.cloud.logging_v2.logger import Logger
from google.cloud.logging_v2.logger import Batch
from google.cloud.logging_v2.metric import Metric
from google.cloud.logging_v2.resource import Resource
from google.cloud.logging_v2.sink import Sink
from google.cloud.logging_v2 import types

__all__ = (
"__version__",
"ASCENDING",
"Batch",
"Client",
"DESCENDING",
"handlers",
"logger_name_from_path",
"Logger",
"LogEntry",
"Metric",
"ProtobufEntry",
"Resource",
"Sink",
"StructEntry",
"TextEntry",
"types",
)
60 changes: 39 additions & 21 deletions google/cloud/logging_v2/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,50 @@

from __future__ import absolute_import

import pkg_resources

try:
__version__ = pkg_resources.get_distribution("google-cloud-logging").version
except pkg_resources.DistributionNotFound:
__version__ = None


from google.cloud.logging_v2.client import Client
from google.cloud.logging_v2.entries import logger_name_from_path
from google.cloud.logging_v2.entries import LogEntry
from google.cloud.logging_v2.entries import TextEntry
from google.cloud.logging_v2.entries import StructEntry
from google.cloud.logging_v2.entries import ProtobufEntry
from google.cloud.logging_v2 import handlers
from google.cloud.logging_v2.logger import Logger
from google.cloud.logging_v2.logger import Batch
from google.cloud.logging_v2.metric import Metric
from google.cloud.logging_v2.resource import Resource
from google.cloud.logging_v2.sink import Sink
from google.cloud.logging_v2 import types
from google.cloud.logging_v2.gapic import config_service_v2_client
from google.cloud.logging_v2.gapic import enums
from google.cloud.logging_v2.gapic import logging_service_v2_client
from google.cloud.logging_v2.gapic import metrics_service_v2_client


class LoggingServiceV2Client(logging_service_v2_client.LoggingServiceV2Client):
__doc__ = logging_service_v2_client.LoggingServiceV2Client.__doc__
enums = enums


class ConfigServiceV2Client(config_service_v2_client.ConfigServiceV2Client):
__doc__ = config_service_v2_client.ConfigServiceV2Client.__doc__
enums = enums


class MetricsServiceV2Client(metrics_service_v2_client.MetricsServiceV2Client):
__doc__ = metrics_service_v2_client.MetricsServiceV2Client.__doc__
enums = enums
ASCENDING = "timestamp asc"
"""Query string to order by ascending timestamps."""
DESCENDING = "timestamp desc"
"""Query string to order by decending timestamps."""


__all__ = (
"enums",
"__version__",
"ASCENDING",
"Batch",
"Client",
"DESCENDING",
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't logging meant to be an alias of logging_v2? Right now it looks like they expose different modules. I can't do logging.logger, but I can do logging_v2.logger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I messed this up.

I think where I'd like to go with this is what Firestore has:

google/cloud/firestore.py (shim)
google/cloud/firestore_v1/__init__.py

The class Client (not the module client) is placed in __all__, so you can import it under logging_v2/logging.

Does that sound alright to you?

from google.cloud import logging

client = logging.Client()
metric = logging.Metric()

This is different from what I have at the current commit (exporting modules) which I don't think makes sense for anything but handlers.

from google.cloud import logging

client = logging.client.Client()
metric = logging.metric.Metric()

Copy link
Contributor

@daniel-sanche daniel-sanche Oct 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah that first one looks good!

What do you think we should do with the auto-generated API (currently in logging_v2)? I want to discourage users from using the auto-generated modules and use it mostly internally, but there may be some gaps in the hand-written code for now, and it may make sense to allow users to "break the glass". Maybe export them as logging.gapic.MetricsServiceV2Client? What do you think?

Also, an issue with from google.cloud import logging is that it collides with the logging standard library, and they are often going to be used together. Right now we tell users to import as import google.cloud.logging and use the full path in their code. I wonder if it would make sense to rename the library to google.cloud.cloudlogging or something so there's no naming conflict

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think we should do with the auto-generated API (currently in logging_v2)? I want to discourage users from using the auto-generated modules and use it mostly internally, but there may be some gaps in the hand-written code for now, and it may make sense to allow users to "break the glass". Maybe export them as logging.gapic.MetricsServiceV2Client? What do you think?

👍 I can remove the generated clients from __all__ and make them available through logging_v2.gapic.MetricServiceV2Client. This is similar to what you have to do to get to the transport layer in microgenerated clients.

>>> from google.cloud import automl
>>> from google.cloud import automl_v1beta1
>>> automl_v1beta1.services.prediction_service.transports.PredictionServiceTransport
<class 'google.cloud.automl_v1beta1.services.prediction_service.transports.base.PredictionServiceTransport'>
>>> automl.services.prediction_service.transports.PredictionServiceTransport
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'google.cloud.automl' has no attribute 'services'

Also, an issue with from google.cloud import logging is that it collides with the logging standard library, and they are often going to be used together. Right now we tell users to import as import google.cloud.logging and use the full path in their code. I wonder if it would make sense to rename the library to google.cloud.cloudlogging or something so there's no naming conflict

Ah I didn't realize that was a problem. This is definitely a good time to do any naming changes. The generator is flexible on namespacing, so it is really up to you.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can remove the generated clients from __all__ and make them available through logging_v2.gapic.MetricServiceV2Client. This is similar to what you have to do to get to the transport layer in microgenerated clients.

Ok, that would be great!

Ah I didn't realize that was a problem. This is definitely a good time to do any naming changes. The generator is flexible on namespacing, so it is really up to you.

Hmm ok, I'll discuss this with the team and get back to you. Let me know if you have strong opinions one way or the other, because I can definitely see pros and cons to each

Copy link
Contributor Author

@busunkim96 busunkim96 Oct 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Alright, I've removed the 3 generated clients from logging_v2/logging. PTAL. Also opened #88 to track a potential naming change.

from google.cloud import logging_v2

logging_v2.gapic.logging_service_v2_client.LoggingServiceV2Client()

Copy link
Contributor

@daniel-sanche daniel-sanche Oct 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It looks good for the most part, but I noticed that some modules are still only accessible through logging_v2 rather than logging. Like google.cloud.logging_v2.sink and google.cloud.logging_v2.metric . Shouldn't the alias be the same for both?

Edit: actually, it looks like the important classes are already exported from each of those files. I think it looks good then!

"handlers",
"logger_name_from_path",
"Logger",
"LogEntry",
"Metric",
"ProtobufEntry",
"Resource",
"Sink",
"StructEntry",
"TextEntry",
"types",
"LoggingServiceV2Client",
"ConfigServiceV2Client",
"MetricsServiceV2Client",
)
6 changes: 3 additions & 3 deletions google/cloud/logging/_gapic.py → google/cloud/logging_v2/_gapic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
from google.protobuf.json_format import MessageToDict
from google.protobuf.json_format import ParseDict

from google.cloud.logging._helpers import entry_from_resource
from google.cloud.logging.sink import Sink
from google.cloud.logging.metric import Metric
from google.cloud.logging_v2._helpers import entry_from_resource
from google.cloud.logging_v2.sink import Sink
from google.cloud.logging_v2.metric import Metric


class _LoggingAPI(object):
Expand Down
8 changes: 4 additions & 4 deletions google/cloud/logging/_helpers.py → google/cloud/logging_v2/_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@

import requests

from google.cloud.logging.entries import LogEntry
from google.cloud.logging.entries import ProtobufEntry
from google.cloud.logging.entries import StructEntry
from google.cloud.logging.entries import TextEntry
from google.cloud.logging_v2.entries import LogEntry
from google.cloud.logging_v2.entries import ProtobufEntry
from google.cloud.logging_v2.entries import StructEntry
from google.cloud.logging_v2.entries import TextEntry

try:
from google.cloud.logging_v2.gapic.enums import LogSeverity
Expand Down
8 changes: 4 additions & 4 deletions google/cloud/logging/_http.py → google/cloud/logging_v2/_http.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
from google.api_core import page_iterator
from google.cloud import _http

from google.cloud.logging import __version__
from google.cloud.logging._helpers import entry_from_resource
from google.cloud.logging.sink import Sink
from google.cloud.logging.metric import Metric
from google.cloud.logging_v2 import __version__
from google.cloud.logging_v2._helpers import entry_from_resource
from google.cloud.logging_v2.sink import Sink
from google.cloud.logging_v2.metric import Metric


class Connection(_http.JSONConnection):
Expand Down
32 changes: 16 additions & 16 deletions google/cloud/logging/client.py → google/cloud/logging_v2/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import os

try:
from google.cloud.logging import _gapic
from google.cloud.logging_v2 import _gapic
except ImportError: # pragma: NO COVER
_HAVE_GRPC = False
_gapic = None
Expand All @@ -28,21 +28,21 @@
import google.api_core.client_options
from google.cloud.client import ClientWithProject
from google.cloud.environment_vars import DISABLE_GRPC
from google.cloud.logging._helpers import _add_defaults_to_filter
from google.cloud.logging._helpers import retrieve_metadata_server
from google.cloud.logging._http import Connection
from google.cloud.logging._http import _LoggingAPI as JSONLoggingAPI
from google.cloud.logging._http import _MetricsAPI as JSONMetricsAPI
from google.cloud.logging._http import _SinksAPI as JSONSinksAPI
from google.cloud.logging.handlers import CloudLoggingHandler
from google.cloud.logging.handlers import AppEngineHandler
from google.cloud.logging.handlers import ContainerEngineHandler
from google.cloud.logging.handlers import setup_logging
from google.cloud.logging.handlers.handlers import EXCLUDED_LOGGER_DEFAULTS

from google.cloud.logging.logger import Logger
from google.cloud.logging.metric import Metric
from google.cloud.logging.sink import Sink
from google.cloud.logging_v2._helpers import _add_defaults_to_filter
from google.cloud.logging_v2._helpers import retrieve_metadata_server
from google.cloud.logging_v2._http import Connection
from google.cloud.logging_v2._http import _LoggingAPI as JSONLoggingAPI
from google.cloud.logging_v2._http import _MetricsAPI as JSONMetricsAPI
from google.cloud.logging_v2._http import _SinksAPI as JSONSinksAPI
from google.cloud.logging_v2.handlers import CloudLoggingHandler
from google.cloud.logging_v2.handlers import AppEngineHandler
from google.cloud.logging_v2.handlers import ContainerEngineHandler
from google.cloud.logging_v2.handlers import setup_logging
from google.cloud.logging_v2.handlers.handlers import EXCLUDED_LOGGER_DEFAULTS

from google.cloud.logging_v2.logger import Logger
from google.cloud.logging_v2.metric import Metric
from google.cloud.logging_v2.sink import Sink


_DISABLE_GRPC = os.getenv(DISABLE_GRPC, False)
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/logging/entries.py → google/cloud/logging_v2/entries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from google.protobuf.json_format import MessageToDict
from google.protobuf.json_format import Parse

from google.cloud.logging.resource import Resource
from google.cloud.logging_v2.resource import Resource
from google.cloud._helpers import _name_from_project_path
from google.cloud._helpers import _rfc3339_nanos_to_datetime
from google.cloud._helpers import _datetime_to_rfc3339
Expand Down
8 changes: 4 additions & 4 deletions google/cloud/logging/handlers/__init__.py → google/cloud/logging_v2/handlers/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

"""Python :mod:`logging` handlers for Google Cloud Logging."""

from google.cloud.logging.handlers.app_engine import AppEngineHandler
from google.cloud.logging.handlers.container_engine import ContainerEngineHandler
from google.cloud.logging.handlers.handlers import CloudLoggingHandler
from google.cloud.logging.handlers.handlers import setup_logging
from google.cloud.logging_v2.handlers.app_engine import AppEngineHandler
from google.cloud.logging_v2.handlers.container_engine import ContainerEngineHandler
from google.cloud.logging_v2.handlers.handlers import CloudLoggingHandler
from google.cloud.logging_v2.handlers.handlers import setup_logging

__all__ = [
"AppEngineHandler",
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/logging/handlers/_helpers.py → google/cloud/logging_v2/handlers/_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
except ImportError: # pragma: NO COVER
flask = None

from google.cloud.logging.handlers.middleware.request import _get_django_request
from google.cloud.logging_v2.handlers.middleware.request import _get_django_request

_DJANGO_TRACE_HEADER = "HTTP_X_CLOUD_TRACE_CONTEXT"
_FLASK_TRACE_HEADER = "X_CLOUD_TRACE_CONTEXT"
Expand Down
6 changes: 3 additions & 3 deletions google/cloud/logging/handlers/app_engine.py → ...e/cloud/logging_v2/handlers/app_engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
import logging
import os

from google.cloud.logging.handlers._helpers import get_trace_id
from google.cloud.logging.handlers.transports import BackgroundThreadTransport
from google.cloud.logging.resource import Resource
from google.cloud.logging_v2.handlers._helpers import get_trace_id
from google.cloud.logging_v2.handlers.transports import BackgroundThreadTransport
from google.cloud.logging_v2.resource import Resource

_DEFAULT_GAE_LOGGER_NAME = "app"

Expand Down
2 changes: 1 addition & 1 deletion ...loud/logging/handlers/container_engine.py → ...d/logging_v2/handlers/container_engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import logging.handlers

from google.cloud.logging.handlers._helpers import format_stackdriver_json
from google.cloud.logging_v2.handlers._helpers import format_stackdriver_json


class ContainerEngineHandler(logging.StreamHandler):
Expand Down
8 changes: 4 additions & 4 deletions google/cloud/logging/handlers/handlers.py → google/cloud/logging_v2/handlers/handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

import logging

from google.cloud.logging.handlers.transports import BackgroundThreadTransport
from google.cloud.logging.logger import _GLOBAL_RESOURCE
from google.cloud.logging_v2.handlers.transports import BackgroundThreadTransport
from google.cloud.logging_v2.logger import _GLOBAL_RESOURCE

DEFAULT_LOGGER_NAME = "python"

Expand Down Expand Up @@ -67,7 +67,7 @@ class CloudLoggingHandler(logging.StreamHandler):

import logging
import google.cloud.logging
from google.cloud.logging.handlers import CloudLoggingHandler
from google.cloud.logging_v2.handlers import CloudLoggingHandler

client = google.cloud.logging.Client()
handler = CloudLoggingHandler(client)
Expand Down Expand Up @@ -135,7 +135,7 @@ def setup_logging(

import logging
import google.cloud.logging
from google.cloud.logging.handlers import CloudLoggingHandler
from google.cloud.logging_v2.handlers import CloudLoggingHandler

client = google.cloud.logging.Client()
handler = CloudLoggingHandler(client)
Expand Down
2 changes: 1 addition & 1 deletion ...d/logging/handlers/middleware/__init__.py → ...ogging_v2/handlers/middleware/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from google.cloud.logging.handlers.middleware.request import RequestMiddleware
from google.cloud.logging_v2.handlers.middleware.request import RequestMiddleware

__all__ = ["RequestMiddleware"]
0 ...ud/logging/handlers/middleware/request.py → ...logging_v2/handlers/middleware/request.py
View file
Open in desktop
File renamed without changes.
Loading