Update the Subscription class

[hernandezc1]

Summary of Changes

Added

• pittgoogle/pubsub.py
  • The Subscription class now supports the arguments: attribute_filter and udf
  • The _create() function for the Subscription class now supports the creation of subscriptions that use Pub/Sub's built-in filters (i.e., filter based on message attributes) and/or single message transforms through user-defined functions

Closes #116.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -0.48%	✅ 25.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (f7201f5)	974	685	70.33%
Head commit (da8ee8c)	985 (+11)	688 (+3)	69.85% (-0.48%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#117)	12	3	25.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[hernandezc1]

@troyraen requesting your review and noting the error caused when building the documentation. Have you encountered that error before?

| CMake Error at CMakeLists.txt:293 (find_package):
    |   By not providing "FindArrow.cmake" in CMAKE_MODULE_PATH this project has
    |   asked CMake to find a package configuration file provided by "Arrow", but
    |   CMake did not find one.
    | 
    |   Could not find a package configuration file provided by "Arrow" with any of
    |   the following names:
    | 
    |     ArrowConfig.cmake
    |     arrow-config.cmake
    | 
    |   Add the installation prefix of "Arrow" to CMAKE_PREFIX_PATH or set
    |   "Arrow_DIR" to a directory containing one of the above files.  If "Arrow"
    |   provides a separate development package or SDK, be sure it has been
    |   installed.
    | 
    | 
    | -- Configuring incomplete, errors occurred!
    | error: command '/usr/local/bin/cmake' failed with exit code 1

Note: This error originates from the build backend, and is likely not a problem with poetry but one of the following issues with pyarrow (19.0.1)

  - not supporting PEP 517 builds
  - not specifying PEP 517 build requirements correctly
  - the build requirements are incompatible with your operating system or Python version
  - the build requirements are missing system dependencies (eg: compilers, libraries, headers).

You can verify this by running pip wheel --no-cache-dir --use-pep517 "pyarrow (==19.0.1)".

Error: Process completed with exit code 1.

[troyraen]

I've seen similar things. They can be a little tricky. But I would think this should be using our poetry.lock file, which we haven't changed in awhile. The most recent change I see that I would suspect could be causing this is #113 which upgraded setuptools for actions/setup-python, but the Build Documentation completed successfully twice after that merged.

I would start by trying to build the documentation locally, if you haven't already. That may or may not provide insight since this could be related to the underlying os environment which will be different for you locally. But it's the easiest thing to do to start looking.

If the local build works for you, I'd suggest upgrading our dependencies in the poetry.lock file. There is a more recent version of pyarrow, so just upgrading that might fix the problem. (But go ahead and upgrade everything.) I think the command is something like poetry update but our docs should say specifically.

[hernandezc1]

@troyraen thanks for your help!

[troyraen]

Thanks! Try to write at least one unit test to check some part of the code you're adding here. (Maybe the lines that parse the function name out of the javascript string?)

Might want to copy the validation piece of #116 into a new issue to be dealt with later.

[troyraen]

Do import google.pubsub_v1.types instead of from ....

Also, is this a new dependency that needs to be added to the toml file?

[troyraen]

You don't need to go into the details here, but do give the user a heads up that both of these strings must conform to very specific requirements and then provide links to the google documentation where they can learn what those requirements are.

In the udf docstring, incorporate "Pub/Sub Single Message Transforms (SMTs)" so it's clear what this is used for.

Check our other docstrings and examples to see what words we typically use to describe message attributes and payloads. New users probably won't immediately understand what they mean and I may have intentionally used different words elsewhere that I thought would be more recognizable. Would be good to be consistent throughout our docs. (Maybe we do use "attributes" and "payload", I just literally don't remember right now.)

[troyraen]

Make it clear that this is simulated data. (Ideally, the topic itself would be called something like lsstsims-loop, but also ok just to make a note right here instead of changing the topic name.)

[troyraen]

Personally, I would name these two variables attribute_filter and udf to match the kwarg names so it's clear from the start where each is destined to be applied. But whatever you prefer.

Add a comment for each explaining what kind of messages will pass the filter. (Does the first one actually do anything with just a field name?)

[troyraen]

Maybe call this variable smt_udf to include the info about what the string is used for?

[troyraen]

Now I see that google calls this variable javascript_udf, so that would be a good name for it here as well. It's helpful to know that this is javascript. Could also add in the SMT info and call it smt_javascript_udf.

[troyraen]

I think this library is quite small, so it's fine to just import it at the top of this file.

[troyraen]

This isn't necessarily a filter though, right? It could be a transformation that doesn't filter.

[troyraen]

I think these should just be kwargs in the touch() function. These are only used once when creating the subscription. And when a user creates a Subscription object for a gcp subscription that already exists, our code doesn't make the filter or udf available in the Subscription object even if they exist on the actual subscription. So I don't think they should be class attributes.

And if the user passes one in but the subscription already exists, should probably raise a warning saying the subscription exists so the kwarg wasn't used.