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

[Question] The rules before and after loading the configuration file are inconsistent. Can they be improved? #3104

Closed
banfg56 opened this issue Nov 30, 2023 · 8 comments · May be fixed by #3132
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested

Comments

@banfg56
Copy link

banfg56 commented Nov 30, 2023

文件第一次加载与监听中加载规则不一致

一、第一次Load配置项的规则

在 config/file/file.go 中行51 会判断当前加载配置配置是目录以及 "." 开头的特殊文件。如果是则忽略

二、使用 fsnotify 监控目录变化后加载规则

在 config/file/watcher.go 行55, 直接加载所有文件

影响与改进

对于部署在Linux上,在线用vim查看配置文件很不友好。.swp 文件报错,而且整个内容都打印出来。是否能做以下改进:
[ ] 配置加载时允许指定配置文件格式,比如:json 、ini等
[ ] .swp类型文件记载规则保持一致

附fsnotify监控目录中vi打开文件场景

Uploading image.png…

@banfg56 banfg56 added the question Further information is requested label Nov 30, 2023
@kratos-ci-bot kratos-ci-bot changed the title [Question] 配置文件加载前后规则不一致,可改进? [Question] The rules before and after loading the configuration file are inconsistent. Can they be improved? Nov 30, 2023
@kratos-ci-bot
Copy link
Collaborator

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


The first loading of the file is inconsistent with the loading rules during monitoring

1. Rules for loading configuration items for the first time

Line 51 in config/file/file.go will determine that the currently loaded configuration configuration is a directory and a special file starting with ".". ignore if yes

2. Use fsnotify to monitor directory changes and load rules after

In config/file/watcher.go line 55, load all files directly

Impact and improvements

For deployment on Linux, it is very unfriendly to use vim to view the configuration file online. .swp file reports an error and the entire content is printed. Can the following improvements be made:
[ ] Allows you to specify the configuration file format when loading the configuration, such as: json, ini, etc.
[ ] .swp type file recording rules remain consistent

Attached is the scene of vi opening files in the fsnotify monitoring directory

Uploading image.png…

@kratos-ci-bot
Copy link
Collaborator

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


Attached is the scene of vi opening a file in the fsnotify monitoring directory
Uploading WechatIMG919.jpg…

@kratos-ci-bot
Copy link
Collaborator

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿


![Uploading WechatIMG919.jpg…] (Attached is the scene of vi opening the file in the fsnotify monitoring directory)

Copy link

dosubot bot commented Nov 30, 2023

🤖

Hello @banfg56,

Nice to meet you! I'm Dosu, a bot here to help with issues related to the Kratos repository. I'm here to assist you with bugs, answer your questions, and guide you on how to become a contributor. While we wait for a human maintainer, feel free to ask me anything. Let's get your issue sorted out together!

您的问题与我在Kratos存储库中找到的一个已解决的问题有些相似。在这个问题配置文件加载如果是文件夹,更新配置重新加载则会报错中,建议的解决方案是避免使用在修改配置文件时会创建临时文件的编辑器。相反,应使用不会创建临时文件的编辑器,如Goland IDE。此外,在修改配置文件之前,确保删除任何现有的临时文件(如yaml~)。

关于您提出的改进建议:

  1. 配置加载时允许指定配置文件格式,比如:json 、ini等:这是一个很好的建议,可以提高Kratos的灵活性和用户体验。然而,作为一个AI,我无法创建拉取请求或问题。我建议您直接在Kratos存储库中提出这个建议。

  2. .swp类型文件记载规则保持一致:这个问题应该可以通过避免使用会创建临时文件的编辑器来解决。如果您仍然遇到问题,我建议您提供更多的信息,如错误消息和您的配置文件,以便我能更好地帮助您。

希望这些信息对您有所帮助。如果您有任何其他问题或需要进一步的澄清,请随时告诉我。

Sources

@shenqidebaozi shenqidebaozi added good first issue Good for newcomers help wanted Extra attention is needed labels Dec 1, 2023
@shenqidebaozi
Copy link
Sponsor Member

@banfg56 This suggestion looks good. We can add a configuration item in the file source to specify which suffixes are allowed to read files. Can you complete it?

@CcCJeaz
Copy link

CcCJeaz commented Dec 18, 2023

kindly to ask if there is somebody working for this issue? If not, I'm glad to undertake it @shenqidebaozi

@shenqidebaozi
Copy link
Sponsor Member

@CcCJeaz ok

@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Mar 21, 2024
Copy link

dosubot bot commented Mar 21, 2024

Hi, @banfg56,

I'm helping the Kratos team manage their backlog and am marking this issue as stale. From what I understand, you opened this issue to address inconsistencies in the rules for loading configuration files before and after the initial load, particularly when using fsnotify to monitor directory changes. There have been discussions and potential solutions offered by various contributors, but the issue remains unresolved.

Could you please confirm if this issue is still relevant to the latest version of the Kratos repository? If it is, please let the Kratos team know by commenting on the issue. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days.

Thank you for your understanding and contributions to Kratos!

@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 28, 2024
@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants