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

Ansys Edb Service as stand alone package #1

Closed
wants to merge 4 commits into from

Conversation

SMoraisAnsys
Copy link
Contributor

@SMoraisAnsys SMoraisAnsys commented Nov 9, 2023

Extract the code related to gRPC Ansys Edb Service from pyedb.
The resulting project should be a stand alone package used as a dependency of pyedb.

Changes from original repo:

  • the protobuf version user is 3.19.3 (downgraded from 3.20) to be consistent with ansys_tools_protoc_helper which strictly depends on 3.19.3
  • the build system leverages ansys_tools_protoc_helper;
  • ignore autogenerated python code (pb2.py and pb2.pyi)
  • update licence year
  • rename package info to use v0 instead of v1 (package renammed ansys.api.edb.v0)
  • import have been rewritten to match import "ansys/api/edb/v0/*.proto"
  • lint associated:
    • material_def.proto: rename materialDef -> material_def; propertyId -> property_id
    • package_def.proto: rename GetTherm_Cond -> GetThermCond; SetTherm_Cond -> SetThermCond; GetTheta_JB -> GetThetaJB; SetTheta_JB -> SetThetaJB; GetTheta_JC -> GetThetaJC; SetTheta_JC -> SetThetaJC

Changes associated to the extraction
- the build system leverages ansys_tools_protoc_helper.
- ignore autogenerated python code (*pb2*.py and *pb2*.pyi)
- update licence year
- rename package info to use v0 instead of v1
Changes associated to the extraction
- package renamged ansys.api.edb.v0
- import have been rewritten to match import "ansys/api/edb/v0/*.proto"
@SMoraisAnsys SMoraisAnsys force-pushed the setup/extract_stand_alone branch 9 times, most recently from 1ecc86a to dd08706 Compare November 9, 2023 13:31
Note
Current protolint step does not leverage the github action
plexsystems/[email protected]. Once the package is released
in public, we should switch to it.
@hiro727
Copy link
Contributor

hiro727 commented Nov 18, 2023

as discussed offline, let's use

  • v1 instead of v0 (requires server side changes)
  • remove lint related changes (requires server side changes)

looks good with

  • protobuf 3.19.3
  • absolute imports

@SMoraisAnsys
Copy link
Contributor Author

@hiro727 Thanks for the feedback, I'll split this PR into two parts (one to be accepted, the other as pending).
Can you send me feedback on ansys/pyedb-core#312 so that I can check both at the same time ?

@drewm102
Copy link
Collaborator

@hiro727 Thanks for the feedback, I'll split this PR into two parts (one to be accepted, the other as pending). Can you send me feedback on ansys/pyedb-core#312 so that I can check both at the same time ?

@SMoraisAnsys Just to double check, did you mean to reference the pull request fix circular imports #313 or the issue Circular import error when importing terminal #312? I wanted to make sure I'm putting my comments in the right place. Thanks!

@SMoraisAnsys
Copy link
Contributor Author

@hiro727 Thanks for the feedback, I'll split this PR into two parts (one to be accepted, the other as pending). Can you send me feedback on ansys/pyedb-core#312 so that I can check both at the same time ?

@SMoraisAnsys Just to double check, did you mean to reference the pull request fix circular imports #313 or the issue Circular import error when importing terminal #312? I wanted to make sure I'm putting my comments in the right place. Thanks!

When hiroki was speaking about sbolute imports I though he was refering to the circular import issue. However that was a mistake. He was speaking about having absolute imports on client side while having something different on server side. Sorry for the confusion :s

@SMoraisAnsys
Copy link
Contributor Author

Closing this PR as it is split between changes accepted #2 and changes to be performed later on #3.

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