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

fix(packaging): python module structure and dependencies #29

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

eclipselu
Copy link
Contributor

@eclipselu eclipselu commented Jan 23, 2021

Tracking issue: #28

  • pin python-dateutil version to 2.8.1
  • added python-dateutil as a dependency in setup.py
  • added __init__.py in every directory under src/
  • fixed README

Tests done:

# see https://packaging.python.org/tutorials/packaging-projects/#generating-distribution-archives
# this generates: google-events-0.1.1.tar.gz  google_events-0.1.1-py2.py3-none-any.whl under dist/
python3 setup.py sdist bdist_wheel

# install the package 
pip3 install dist/google_events-0.1.1-py2.py3-none-any.whl

# import successfully
Python 3.9.1 (default, Jan  5 2021, 14:05:22)
[Clang 12.0.0 (clang-1200.0.32.27)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from google.events.cloud.pubsub.v1 import MessagePublishedData
>>> MessagePublishedData
<class 'google.events.cloud.pubsub.v1.MessagePublishedData.MessagePublishedData'>
>>>

@eclipselu eclipselu requested a review from a team as a code owner January 23, 2021 03:16
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 23, 2021
@product-auto-label product-auto-label bot added the api: eventarc Issues related to the googleapis/google-cloudevents-python API. label Jan 23, 2021
@grant
Copy link
Contributor

grant commented Jan 23, 2021

Thanks! We'll check back after the weekend

@eclipselu
Copy link
Contributor Author

Not sure if we need to change the generator too. Is there any instructions on how to use it?

@grant
Copy link
Contributor

grant commented Jan 25, 2021

Not sure if we need to change the generator too. Is there any instructions on how to use it?

It looks like this repo needs instructions for building the library.

Here's the gen script:

https://github.com/googleapis/google-cloudevents-python/blob/master/gen.sh

Here's the core generator:

https://github.com/googleapis/google-cloudevents/tree/master/tools/quicktype-wrapper

@eclipselu
Copy link
Contributor Author

eclipselu commented Jan 25, 2021

Thanks! but the gen script does not seem to work on my side:

Generating the Google Events Library for Python...

> gen@1.0.0 start
> node build/src
- Finding all JSON Schemas (*.json)...
/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/node_modules/urijs/src/URI.js:54
        throw new TypeError('undefined is not a valid argument for URI');
              ^

TypeError: undefined is not a valid argument for URI
    at new URI (/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/node_modules/urijs/src/URI.js:54:15)
    at JSONSchemaInput.addSourceSync (/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/node_modules/quicktype-core/dist/input/JSONSchemaInput.js:1018:31)
    at JSONSchemaInput.<anonymous> (/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/node_modules/quicktype-core/dist/input/JSONSchemaInput.js:1008:25)
    at Generator.next (<anonymous>)
    at /Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/node_modules/quicktype-core/dist/input/JSONSchemaInput.js:7:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/node_modules/quicktype-core/dist/input/JSONSchemaInput.js:3:12)
    at JSONSchemaInput.addSource (/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/node_modules/quicktype-core/dist/input/JSONSchemaInput.js:1007:16)
    at quicktypeJSONSchemaToMultiFile (/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/build/src/quickstype.js:93:23)
    at Object.jsonschema2languageFiles (/Users/danglu/Downloads/google-cloudevents-python/gen/quicktype-wrapper/build/src/quickstype.js:116:35)
npm ERR! code 1
npm ERR! path /Users/danglu/Downloads/google-cloudevents-python/gen
npm ERR! command failed
npm ERR! command sh -c node build/src

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/danglu/.npm/_logs/2021-01-25T22_59_52_427Z-debug.log

@grant
Copy link
Contributor

grant commented Jan 26, 2021

@eclipselu I have found an upstream issue that is contributing to this error on the head branch.

Here is the fix for that: googleapis/google-cloudevents#145

After that fix is in, that generator script shouldn't see the same error.

Thanks for your patience.


@michaelawyu can you please look at this PR? I'm not sure if the generator is supposed to use a specific version. A full review of this PR is needed.

gcf-merge-on-green bot pushed a commit to googleapis/google-cloudevents that referenced this pull request Jan 26, 2021
This PR ensures the JSON schema `catalog.json` file isn't pulled in as a JSON schema input file.

Related: googleapis/google-cloudevents-python#29
@eclipselu
Copy link
Contributor Author

eclipselu commented Jan 26, 2021

@grant thanks for the fix.

@michaelawyu I have changed gen/ to generate the __init__.py files, but the actual python class has some problems, the content of the classes are all stdout, for example:

$ cat src/google/events/cloud/audit/v1/LogEntryData.py
# Copyright 2020 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# 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.
stdout

I did not commit these files. It seems in https://github.com/googleapis/google-cloudevents/blob/master/tools/quicktype-wrapper/src/index.ts#L102, it can be changed to:

- schemasAndGenFiles.push([schema, Object.keys(genFile)[0]]);
+ schemasAndGenFiles.push([schema, Object.values(genFile)[0]]);

Please take a look, thanks!

@michaelawyu
Copy link
Contributor

michaelawyu commented Jan 26, 2021

Hi @eclipselu,

Thanks for the PR! Since this issue requires some tweaking on the generation script as well I have sent another PR to fix this: #32. PTAL.

@eclipselu eclipselu mentioned this pull request Jan 26, 2021
- pin python-dateutil version to 2.8.1
- added python-dateutil as a dependency in setup.py
- added `__init__.py` in every directory
- fixed README
Copy link
Contributor

@grant grant left a comment

Choose a reason for hiding this comment

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

I've reviewed the files. The PR looks good to me.

@grant grant added the automerge Merge the pull request once unit tests and other checks pass. label Feb 22, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit 0d3b38b into googleapis:master Feb 22, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Feb 22, 2021
gcf-merge-on-green bot pushed a commit that referenced this pull request Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: eventarc Issues related to the googleapis/google-cloudevents-python API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants