You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2024/03/18 17:46:09 UTC
[PR] feat: config codegen [opendal]
tisonkun opened a new pull request, #4378:
URL: https://github.com/apache/opendal/pull/4378
* This closes #4278.
* This refers to #4279.
Let's see if we'd like to go in this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4378:
URL: https://github.com/apache/opendal/pull/4378#issuecomment-2006820973
Follow up -
* Tracking issues to convert other configs in this pattern.
* Work on generate code for Java / Python / ...
cc @Xuanwo @PsiACE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530194849
##########
codegen/template/config.rs.j2:
##########
Review Comment:
I think we can have `core/src/service/${service}/generated.rs`. It's also fine to me to have `core/src/service/${service}/config.generated.rs`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #4378:
URL: https://github.com/apache/opendal/pull/4378#issuecomment-2005757343
I don't find writing in Go appealing, especially since almost all of our dev related scripts are written in python.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530180546
##########
codegen/template/config.rs.j2:
##########
Review Comment:
This is possible. So we keep generate the S3 config to `core/src/service/s3/?`.
Maybe you have some inputs about the layout. I'm open to this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530175164
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
> I want to avoid adding any new dependencies.
This is strange. It means we regenerate many utilities by hand.
Code generator isn't such a trivial thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530178255
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
> Or we can make scripts as a whole new package that managed as a real python project.
This is possible. Or call it `tools`/`pytools`. I suppose we'd have some handy scripts may be not in Python (or Python is not a good fit).
Firstly I'd make a consensus that write a code generator without any 3rdparty will end up with write every utility by hand and it's even .. stupid.
Then we back to the naming issue about how we call the current "codegen" folder.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530197452
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
> Firstly I'd make a consensus that write a code generator without any 3rdparty will end up with write every utility by hand and it's even .. stupid.
Agrees.
> Then we back to the naming issue about how we call the current "codegen" folder.
We can merge current `scripts` and `codegen` into one `tools` or `odev`.
And maybe we can use the same entry point: `odev`?
Having a `odev --help` maybe make it easier to discover?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun closed pull request #4378: feat: config codegen
URL: https://github.com/apache/opendal/pull/4378
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530175164
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
> I want to avoid adding any new dependencies.
This is strange. It means we recreate many utilities by hand.
Code generator isn't such a trivial thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4378:
URL: https://github.com/apache/opendal/pull/4378#issuecomment-2006255815
@Xuanwo finished to Python.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4378:
URL: https://github.com/apache/opendal/pull/4378#issuecomment-2005765223
@Xuanwo think of it a typed Python script. Better than untyped toml strings..
But maybe I can port all these things into Python with Jinja template or sth.
Just no untyped toml, we can write the schema in code.
Newer Python can be typed also.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530197452
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
> Firstly I'd make a consensus that write a code generator without any 3rdparty will end up with write every utility by hand and it's even .. stupid.
Agrees.
> Then we back to the naming issue about how we call the current "codegen" folder.
We can merge current `scripts` and `codegen` into one `tools` or `odev`.
And maybe we can use the same entry point: `odev`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1529995720
##########
codegen/src/codegen/config/config.py:
##########
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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
+#
+# http://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.
+
+import dataclasses
+import enum
+import textwrap
+from typing import List
+
+import humps
+
+
+@enum.unique
+class FieldType(enum.Enum):
+ Nothing = enum.auto()
+ Bool = enum.auto()
+ Int = enum.auto()
+ Long = enum.auto()
+ Float = enum.auto()
+ Double = enum.auto()
+ String = enum.auto()
+
+
+@enum.unique
+class RefinedFieldType(enum.Enum):
+ Nothing = enum.auto()
+ RustUsize = enum.auto()
+
+
+@dataclasses.dataclass
+class ConfigField:
+ name: str
+ desc: str
+
+ ty: FieldType = FieldType.Nothing
+ refined_ty: RefinedFieldType = RefinedFieldType.Nothing
+
+ required: bool = False
+ sensitive: bool = False
+
+ # default value for the config field if not set
+ default: str = ""
+ # a list of available values for the config field
+ available: List[str] = dataclasses.field(default_factory=list)
+ # a list of example values for the config field
+ example: List[str] = dataclasses.field(default_factory=list)
+
+ ###########
+ # RustGen #
+ ###########
+ def rust_type(self) -> str:
Review Comment:
I don't know what's the idiom to group methods. In Golang, receiver methods can scatter in multiple files. In Python, perhaps we need some design pattern like "XlangConfig(Field)Generator" and take "Config(Field)" a parameter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530162377
##########
codegen/template/config.rs.j2:
##########
Review Comment:
I plan to generate more codes for `S3Builder`. Therefore, I prefer to maintain them separately in each service rather than composing everything into one large `config.rs`.
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
I'm opposed to installing tools like `pdm`. In fact, I want to avoid adding any new dependencies. We don't need to package/release/dist it. I expect this code generation can be executed with a simple `python3 xxx.py`, just like all other scripts. Thus I prefer to keep it inside `scripts`.
Or we can make `scripts` as a whole new package that managed as a real python project.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4378:
URL: https://github.com/apache/opendal/pull/4378#discussion_r1530199173
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
> And maybe we can use the same entry point: `odev`?
This is much furthur on current codebase. We can start with `tools` as a python project first.
##########
codegen/README.md:
##########
@@ -0,0 +1,21 @@
+# Codegen
+
+## Generate
+
+### Generate service config files
+
+Run the following command to generate service config files:
+
+```bash
+pdm genconfig
Review Comment:
> And maybe we can use the same entry point: `odev`?
This is much furthur on current codebase. We can start with treating `tools` as a python project first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: config codegen [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4378:
URL: https://github.com/apache/opendal/pull/4378#issuecomment-2008573438
`scripts/` is written with a mindset used standalone.
Forcely combine it with our current work is strange.
I don't want to do this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org